Cache busting config reloading by using virtual files #204
Conversation
|
You can download the latest build of the extension for this PR here: To install the extension, download the file, unzip it and install it in VS Code by selecting "Install from VSIX..." in the Extensions view. Alternatively, run code --install-extension vscode-apollo-0.0.0-build-1726589871.pr-204.commit-a5c9f80.vsix --forcefrom the command line. For older builds, please see the edit history of this comment. |
phryneas
changed the title
phryneas
force-pushed
the
pr/config-cachebusting
branch
from
e66e5e3 to
39115aa
Compare
| if (uri.match(/vscode-extension-ignore.[mc]?js$/)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This is no longer required since we don't create files on disk anymore
| { | ||
| with: { | ||
| as: "cachebust", | ||
| contents: transpiledContent, | ||
| format: type, | ||
| } satisfies ImportAttributes, | ||
| } |
There was a problem hiding this comment.
This will be passed to our custom resolver, the file path will be largely ignored.
phryneas
changed the title
| module.exports = { | ||
| client: { | ||
| service: { | ||
| name: "localSchema", | ||
| localSchemaFile: "./starwarsSchema.graphql", | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
We need one "TypeScript, but CommonJS" config for testing, and the rover config seemed like a better candidate for yaml anyways
phryneas
force-pushed
the
pr/config-cachebusting
branch
from
605eb83 to
e4ae818
Compare
| * @returns {Promise<ResolutionResult>} | ||
| */ | ||
| async function resolve(specifier, context, nextResolve) { | ||
| if (context.importAttributes.as !== "cachebust") { |
There was a problem hiding this comment.
It was context.importAssertions. So, in some older versions of Node.js, context.importAttributes will be undefined. Unfortunately, Cursor is using Node.js 20.9.0 in their latest version.
See, https://nodejs.org/api/module.html#hooks:~:text=v21.0.0%2C%20v20.10.0%2C%20v18.19.0
| filepath, | ||
| //@ts-ignore | ||
| { | ||
| with: { |
There was a problem hiding this comment.
I’m trying to fix #225 and found out the issue is not only related to importAttributes. The with keyword is also newly introduced in new node versions.
There was a problem hiding this comment.
As these are object keys, what happens if you pass this in as an object with both the with and assert property?
There was a problem hiding this comment.
It seems fine to pass both with and assert to the import function. But I found we won't be able to access the format key from importAssertions. importAssertions only includes two fields despite we setting other keys on the assert object.
There was a problem hiding this comment.
Hmmm :/
Maybe we need to switch to as: 'cachebust:module' and as: 'cachebust:commonjs' then - or does it maybe work with a different key name than format? Maybe that is in some way reserved?
We needed a way to get around node's
importcache, so this seems to be the best way to do that. Setting a module-path query parameter doesn't seem to have any effect.On a positive sidenote, we also don't have to write any files to disk for TS transpilation anymore.