-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Set impliedNodeFormat
in every module mode
#57570
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
Conversation
@typescript-bot perf test |
Heya @andrewbranch, I've started to run the diff-based top-repos suite on this PR at 74fc5a5. You can monitor the build here. Update: The results are in! |
Heya @andrewbranch, I've started to run the regular perf test suite on this PR at 74fc5a5. You can monitor the build here. Update: The results are in! |
@andrewbranch Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@andrewbranch Here are the results of running the top-repos suite comparing Everything looks good! |
74fc5a5
to
ab5d376
Compare
@typescript-bot perf test |
Heya @andrewbranch, I've started to run the regular perf test suite on this PR at ab5d376. You can monitor the build here. Update: The results are in! |
@andrewbranch Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
@typescript-bot test top200 |
Starting jobs; this comment will be updated as builds start and complete.
|
@andrewbranch Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
The pipeline actually failed to push due to the text being too long; will try and fix that, but it was about to send a looot of comments: https://typescript.visualstudio.com/TypeScript/_build/results?buildId=160202&view=logs&j=15ac8a4d-b341-5815-5352-14d7ae9c5a86&t=b4a5e2cd-8727-5d20-e3d3-86822d3c46a6 |
@typescript-bot test top100 |
@andrewbranch Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
import referencedSource from "../../lib/src/a"; // Error | ||
import referencedDeclaration from "../../lib/dist/a"; // Error |
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.
@sheetalkamat do you think it’s worth it to fix this? These two default imports would ideally error, because we could know by using the referenced project settings that the emit was ESM. We currently don’t error because we assume that a declaration file that says export declare const a = 0
could represent a CJS file. I’m not sure if it should be out of scope for this PR to start using referenced project settings to get more accurate information during type checking like this.
@typescript-bot test top200 |
@typescript-bot test top200 |
@andrewbranch Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
This is a starting point for substantive changes, only up in this state to test perf and breaking changes.