Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Use LSP based file watcher #1610

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions docs/user/config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ There's no support for multiple config files or inheriting/overriding config fil
"language": "Default",
"permissionLevel": 2,
"plugins": [],
"mcmetaSummaryOverrides": {},
"useFilePolling": false
"mcmetaSummaryOverrides": {}
},
"format": {
"blockStateBracketSpacing": { "inside": 0 },
Expand Down
30 changes: 24 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
"docs:start": "cd docs && (bundle exec jekyll serve --incremental || true) && cd ..",
"fmt": "dprint fmt",
"fmt:check": "dprint check",
"lint": "eslint --cache --max-warnings=0 packages/*/{src,test}/**/*.{cts,mts,ts}",
"lint:fix": "eslint --cache --max-warnings=0 --fix packages/*/{src,test}/**/*.{cts,mts,ts}",
"lint": "eslint --cache --max-warnings=0 \"packages/*/{src,test}/**/*.{cts,mts,ts}\"",
"lint:fix": "eslint --cache --max-warnings=0 --fix \"packages/*/{src,test}/**/*.{cts,mts,ts}\"",
"release": "ts-node scripts/release.ts",
"release:dry": "ts-node scripts/release.ts --dry-run",
"set-tsconfig-references": "ts-node scripts/set_tsconfig_references",
Expand Down
1 change: 0 additions & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
"dependencies": {
"base64-arraybuffer": "^1.0.2",
"binary-search": "^1.3.6",
"chokidar": "^3.5.2",
"decompress": "^4.2.1",
"follow-redirects": "^1.14.8",
"micromatch": "^4.0.8",
Expand Down
29 changes: 1 addition & 28 deletions packages/core/src/common/externals/BrowserExternals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,7 @@ import type {
ExternalDownloaderOptions,
RemoteUriString,
} from './downloader.js'
import type {
ExternalEventEmitter,
ExternalFileSystem,
Externals,
FsLocation,
FsWatcher,
} from './index.js'
import type { ExternalEventEmitter, ExternalFileSystem, Externals, FsLocation } from './index.js'

type Listener = (...args: unknown[]) => unknown
export class BrowserEventEmitter implements ExternalEventEmitter {
Expand Down Expand Up @@ -71,24 +65,6 @@ class BrowserExternalDownloader implements ExternalDownloader {
}
}

class BrowserFsWatcher implements FsWatcher {
on(event: string, listener: (...args: any[]) => unknown): this {
if (event === 'ready') {
listener()
}
return this
}

once(event: unknown, listener: (...args: any[]) => unknown): this {
if (event === 'ready') {
listener()
}
return this
}

async close(): Promise<void> {}
}

class BrowserFileSystem implements ExternalFileSystem {
private static readonly LocalStorageKey = 'spyglassmc-browser-fs'
private states: Record<string, { type: 'file'; content: string } | { type: 'directory' }>
Expand Down Expand Up @@ -149,9 +125,6 @@ class BrowserFileSystem implements ExternalFileSystem {
delete this.states[location]
this.saveStates()
}
watch(_locations: FsLocation[]): FsWatcher {
return new BrowserFsWatcher()
}
async writeFile(
location: FsLocation,
data: string | Uint8Array,
Expand Down
30 changes: 1 addition & 29 deletions packages/core/src/common/externals/NodeJsExternals.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/60592
import chokidar from 'chokidar'
import decompress from 'decompress'
import followRedirects from 'follow-redirects'
import { Buffer } from 'node:buffer'
Expand All @@ -19,7 +18,7 @@ import type {
ExternalDownloaderOptions,
RemoteUriString,
} from './downloader.js'
import type { Externals, FsLocation, FsWatcher } from './index.js'
import type { Externals, FsLocation } from './index.js'

const { http, https } = followRedirects
const gunzip = promisify(zlib.gunzip)
Expand Down Expand Up @@ -114,14 +113,6 @@ export const NodeJsExternals: Externals = {
unlink(location) {
return fsp.unlink(toFsPathLike(location))
},
watch(locations, { usePolling = false } = {}) {
return new ChokidarWatcherWrapper(
chokidar.watch(locations.map(toPath), {
usePolling,
disableGlobbing: true,
}),
)
},
writeFile(location, data, options) {
return fsp.writeFile(toFsPathLike(location), data, options)
},
Expand Down Expand Up @@ -154,22 +145,3 @@ function toPath(path: FsLocation): string {

const uriToPath = (uri: string | Uri) =>
url.fileURLToPath(uri instanceof Uri ? new url.URL(uri.toString()) : uri)
const uriFromPath = (path: string) => url.pathToFileURL(path).toString()

class ChokidarWatcherWrapper extends EventEmitter implements FsWatcher {
readonly #watcher: chokidar.FSWatcher

constructor(watcher: chokidar.FSWatcher) {
super()
this.#watcher = watcher
.on('ready', () => this.emit('ready'))
.on('add', (path) => this.emit('add', uriFromPath(path)))
.on('change', (path) => this.emit('change', uriFromPath(path)))
.on('unlink', (path) => this.emit('unlink', uriFromPath(path)))
.on('error', (e) => this.emit('error', e))
}

close(): Promise<void> {
return this.#watcher.close()
}
}
20 changes: 0 additions & 20 deletions packages/core/src/common/externals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export interface ExternalFileSystem {
showFile(path: FsLocation): Promise<void>
stat(location: FsLocation): Promise<{ isDirectory(): boolean; isFile(): boolean }>
unlink(location: FsLocation): Promise<void>
watch(locations: FsLocation[], options: { usePolling?: boolean }): FsWatcher
/**
* @param options `mode` - File mode bit mask (e.g. `0o775`).
*/
Expand All @@ -87,22 +86,3 @@ export interface ExternalFileSystem {
* A file URI string or a URI object.
*/
export type FsLocation = string | Uri

export interface FsWatcher {
on(eventName: 'ready', listener: () => unknown): this
once(eventName: 'ready', listener: () => unknown): this

on(eventName: 'add', listener: (uri: string) => unknown): this
once(eventName: 'add', listener: (uri: string) => unknown): this

on(eventName: 'change', listener: (uri: string) => unknown): this
once(eventName: 'change', listener: (uri: string) => unknown): this

on(eventName: 'unlink', listener: (uri: string) => unknown): this
once(eventName: 'unlink', listener: (uri: string) => unknown): this

on(eventName: 'error', listener: (error: Error) => unknown): this
once(eventName: 'error', listener: (error: Error) => unknown): this

close(): Promise<void>
}
11 changes: 0 additions & 11 deletions packages/core/src/service/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,6 @@ export interface EnvConfig {
* May become corrupt after changing game versions, so this is currently disabled by default.
*/
enableMcdocCaching: boolean
/**
* Makes the file-watcher use polling to watch for file changes.
* Comes at a performance cost for very large datapacks.
*
* On Windows, enabling this can fix file-lock issues when Spyglass is running.
* See: https://github.com/SpyglassMC/Spyglass/issues/1414
*
* **You should only consider enabling this for Windows machines.**
*/
useFilePolling: boolean
}

export type LinterSeverity = 'hint' | 'information' | 'warning' | 'error'
Expand Down Expand Up @@ -369,7 +359,6 @@ export const VanillaConfig: Config = {
plugins: [],
mcmetaSummaryOverrides: {},
enableMcdocCaching: false,
useFilePolling: false,
},
format: {
blockStateBracketSpacing: { inside: 0 },
Expand Down
Loading
Loading