Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add auto-import for the
package.json
imports
field #55015Add auto-import for the
package.json
imports
field #55015Changes from 5 commits
7c0fef4
60ec33b
faef5fd
1d49909
4e676a3
747e769
01ceb65
864b9a3
d0407a9
e092943
8f781c5
a2aa5ae
eaa8e2e
051239b
b3c5ca4
ea67766
7abf4a2
6a72a51
9903d72
9e2df94
c4df857
566e543
1f9f214
7a5c164
45194b2
8f2d0c3
8bef690
cd84162
8d68392
a9634ed
7f52af2
4aab191
c435528
4f29d3a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I don't think this is a new problem as this file already has code that looks exactly like this, but I'm somewhat certain that we should be using
readJson
for this; if the JSON is malformed, we'll throw here and then I don't know what will happen.readJson
is what's used to get stuff into the cache, and generally speaking other rawJSON.parse
calls all appear to at leasttry/catch
.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.
I see that there's another thread about this at #55015 (comment); if we want to leave this
JSON.parse
in here given we already do that, that's fine, but overall if we're avoidingreadJson
because it calls the wrong JSON parser, but the alternatiive is to callJSON.parse
unguarded... that feels spooky to me.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.
@andrewbranch should this autocomplete#something.js
? I'm investigating some stuff and I concluded the conclusion that perhaps this should actually be just#something
.@module: nodenext
doesn't prescribe the specifier ending - and since thispackage.json
doesn't have a type it's an implied CJS file so the ending is optional.note for myself: fixing this would likely depend on usinggetModuleSpecifierEndingPreference
appropriatelyHmm, I still observe some inconsistencies elsewhere but now I concluded that this one is correct -
imports
/exports
always require extensions (regardless of the module format).