Port --traceResolutions #1537
Port --traceResolutions #1537
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR ports the TypeScript --traceResolutions feature to the Go implementation, which provides detailed logging of module resolution steps during compilation. This feature is particularly useful for debugging module resolution issues.
Key changes:
- Added trace resolution logging to the module resolution system
- Implemented detailed resolution step tracking with conditional output
- Updated baseline files to include new trace output format
Reviewed Changes
Copilot reviewed 300 out of 368 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
testdata/baselines/reference/submodule/conformance/*.trace.json |
Reference baseline files showing expected trace output |
testdata/baselines/reference/submodule/conformance/*.trace.json.diff |
Diff files showing changes in trace output behavior between old and new implementations |
sheetalkamat
force-pushed
the
traceResolutions
branch
from
9f4ebc4 to
2b7119a
Compare
| File '/packages/main/node_modules/shared/index.d.ts' does not exist. | ||
| Directory '/packages/main/node_modules/@types' does not exist, skipping all lookups in it. | ||
| Directory '/packages/node_modules' does not exist, skipping all lookups in it. | ||
| +Directory '/packages/node_modules/@types' does not exist, skipping all lookups in it. |
There was a problem hiding this comment.
seems like we are adding these extra lookups
There was a problem hiding this comment.
All of this caching was removed, because the FS itself caches. We just let the resolver go wild.
| Directory '/node_modules/@types' does not exist, skipping all lookups in it. | ||
| -Resolving real path for '/packages/main/node_modules/shared/index.js', result '/packages/shared/index.js'. | ||
| -======== Module name 'shared' was successfully resolved to '/packages/shared/index.js' with Package ID 'shared/[email protected]'. ======== | ||
| +======== Module name 'shared' was successfully resolved to '/packages/shared/index.js' with Package ID '[email protected]'. ======== |
There was a problem hiding this comment.
package id seems to be different
There was a problem hiding this comment.
I suspect this comes down us not having ported classic/node10 resolution
| File '/node_modules/pkg/index.d.ts' exists - use it as a name resolution result. | ||
| Resolving real path for '/node_modules/pkg/index.d.ts', result '/node_modules/pkg/index.d.ts'. | ||
| ======== Module name 'pkg' was successfully resolved to '/node_modules/pkg/index.d.ts'. ======== | ||
| -File '/node_modules/pkg/package.json' does not exist according to earlier cached lookups. |
There was a problem hiding this comment.
i didnt add traces for package .json lookups for file's metadata that we use to in strada as part of this PR
|
I suspect diffing the resolutions won't be useful due to the things noted above; we don't have classic/node10 resolution and we removed all deduplication from the resolver because it was far faster to just let the FS cache things. |
So do we not want to diff with old but baseline |
|
I am not sure; I think we had been concerned about the output even being consistent given it's now happening concurrently. It implies that the resolutions can't be streamed to an output without being interleaved. |
The way this is implemented it guaranetees the order of the trace i am not sure if diffing with old is useful or not and who is looking at it so may be i will just skip that part but baseline the trace so we can detect the changes |
| @@ -42,7 +43,7 @@ func Run(t *testing.T, fileName string, actual string, opts Options) { | |||
| writeComparison(t, actual, localPath, referencePath, false) | |||
| } | |||
|
|
|||
| if !opts.IsSubmodule { | |||
| if !opts.IsSubmodule || opts.SkipDiffWithOld { | |||
There was a problem hiding this comment.
Is this still needed? I think it's leftover now that the diffs were dropped.
There was a problem hiding this comment.
This is needed to ensure that it does not diff with old
Should i be setting "submodule" as false instead but then the trace will go in different location compared to their emit and other stuff ?
There was a problem hiding this comment.
Hm, you're right. I thought we had a way to do this before but I'm not seeing it anymore.
No description provided.