GeezerVison is showing github.com — Full support for CommonJS auto-imports in JS by andrewbranch · Pull Request #37027 · microsoft/TypeScript
Conversation
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 and require for a JS file?
Rules are evaluated in order (highest priority first).
- If the imported identifier could be added to an existing
import, add it there.
- If the imported identifier could be added to an existing
require, add it there.
- If the importing file contains any ES2015+ import/export syntax, use
import.
- If the importing file contains any CommonJS syntax, use
require.
- If the compiler options indicate a module target of ES2015+, use
import.
- Use
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:
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
nykula, florian-milky, peterblazejewicz, avevlad, bbugh, and JonathanLee-LX reacted with thumbs up emoji
bbugh reacted with hooray emoji
bbugh reacted with heart emoji
All reactions
-
👍
6 reactions
-
🎉
1 reaction
-
❤️
1 reaction
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
-173
to
+177
|
|
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 mean import foo = require('mod') or const foo = require('mod')
Namespace can mean import * as foo from 'mod' or const foo = require('mod')
Default can mean import foo from 'mod' or const { default: foo } = require('mod')
Named can mean import { foo } from 'mod' or const { foo } = require('mod')
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
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.
|