-
Notifications
You must be signed in to change notification settings - Fork 21
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
Avoid detection if .js config file is ESM or CommonJs, just try both. #211
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-1726752181.pr-211.commit-24c135b.vsix --force from the command line. For older builds, please see the edit history of this comment. |
url: bustFileName(specifier), | ||
format: context.importAttributes.format, | ||
importAttributes: context.importAttributes, | ||
shortCircuit: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't trust the original detection algorithm at all anymore, format
is required now and we just pass this in from the outside.
@@ -0,0 +1,4 @@ | |||
{ | |||
"name": "test", | |||
"type": "commonjs" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deliberately specifying the opposite here - the config file is .js
with ESM contents, the package here has type: commonjs
, we still want it to work.
The E2E tests will try all possible combinations on top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me!
This came up here: #187 (comment)
Because we relied on the original
resolve
call to determine if a file was a module or commonjs, that call would apply the normal ESM detection logic - if yourpackage.json
hadtype: 'module'
, this means that your config file could not be CommonJS anymore.This approach now just tries to load it as ESM and if that fails tries again with CJS - we do the same for TypeScript already.