-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Full support for CommonJS auto-imports in JS #37027
Full support for CommonJS auto-imports in JS #37027
Conversation
Equals, | ||
ConstEquals | ||
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.
This is the fundamental conceptual change in this file, and everything else is basically just implementation details. This enum used to represent final syntax; now it’s more a representation of where the symbol can be found in the module. Whether or not to use JS is then calculated and stored elsewhere, so that
CommonJS
can meanimport foo = require('mod')
orconst foo = require('mod')
Namespace
can meanimport * as foo from 'mod'
orconst foo = require('mod')
Default
can meanimport foo from 'mod'
orconst { default: foo } = require('mod')
Named
can meanimport { foo } from 'mod'
orconst { foo } = require('mod')
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.
Looks pretty good to me, although I wasn't previously familiar with the importFixes code.
Fix typos Co-Authored-By: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
In #32684, I added the ability for auto-imports to write a
require
variable declaration in JS files. However, it didn’t work for named or default exports, and under default project settings, would never be triggered from completions; it was only reachable by code fix. This is particularly problematic for JS, because unless the user has explicitly enabled type checking JS through VS Code settings, a tsconfig or jsconfig file, or// @ts-check
, you can’t get code fixes because you don’t get errors.This PR removes those limitations, and also recognizes members of object literals assigned to
module.exports
as importable symbols.How do we choose between
import
andrequire
for a JS file?Rules are evaluated in order (highest priority first).
import
, add it there.require
, add it there.import
.require
.import
.require
.How are
require
variable declarations written for different kinds of exports?const { foo } = require('./foo')
maps to named ES exports and CommonJS exported object literal members (module.exports = { foo }
).const { default: foo } = require('./foo')
maps to default ES exports (hopefully people aren’t actually doing this).const foo = require('./foo')
maps to everything else.Fixes #20292
Fixes #30549
Related: