lifeart / ember-language-server

Language Server Protocol implementation for Ember.js and Glimmer projects
https://marketplace.visualstudio.com/items?itemName=lifeart.vscode-ember-unstable
MIT License
32 stars 16 forks source link

figure out caching issue: why template-lint config changes not applyed to next hbs linting #142

Open lifeart opened 4 years ago

lifeart commented 4 years ago

Steps to reproduce:

1.) open any hbs file, with linting issue 2.) disable rule in template-lint-config 3.) open this file one more time

Expected behavour:

Existing bahavour:

saravanak commented 4 years ago

Had a look on this, the reason is that the template file (and its footprint) is cached in two locations

I evicted (1) by deleting the cache entry on the onDidChangeContent on the document listener , when a ember-cli-build.js file. But for (2), the only way to evict is probably doing a delete require.cache[...] or by using a library (decache ?) for it

I tried to emulate the CLIEngine way of eslint, but the exports in get_config.js have changed quite a bit, so not sure if we can reliably create a wrapper similar to CLIEngine for ember-template-lint

What are your thoughts on this @lifeart

lifeart commented 4 years ago

looks like resolved config cached inside template-lint itself https://github.com/ember-template-lint/ember-template-lint/blob/e17ac93f43f3aed3c16f734ee87c84c52d65dfcc/lib/get-config.js#L481, by require in resolveProjectConfig.

and we can follow few options:

@saravanak, @NullVoxPopuli I would prefer to try run linter in new node process, any conserns?

saravanak commented 4 years ago

I am for

run linter inside fork process

I think that if we follow a de-bounced aggregation of the files changed, and then fork the linter (for that set of files), it would be great. I saw some references in vscode-eslint, and they have an internal queue that is being used for the linting process.

https://github.com/microsoft/vscode-eslint/blob/e0f67ac57e757aeb48017b8da6667123c0dc5cff/server/src/eslintServer.ts#L866

Can we use a similar approach here? Also not sure of the rate of change of files from the onDidChangeContent callback for standard editor actions.

lifeart commented 4 years ago

I think we can start from simpler promise-based subprocess api (to have async/await syntax), without extra de-bounce/queue and figure out more complex path if needed.

avg rate is about 4-8rps

lifeart commented 4 years ago

in future, we can introduce debounce in this place: https://github.com/lifeart/ember-language-server/blob/component-context-info-origin/src/server.ts#L368, but I think we need to check debounce-less implementation first

lifeart commented 4 years ago

https://github.com/piscinajs/piscina may be used https://github.com/mantoni/eslint_d.js https://github.com/sindresorhus/execa