Skip to content
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

Support module_resolution: "nodenext" #8748

Merged
merged 13 commits into from
Jul 29, 2024

Conversation

mischnic
Copy link
Contributor

@mischnic mischnic commented Jul 15, 2024

Description

Allow resolving for example ./foo.js to ./foo.ts

.js can resolve to .ts and .tsx
.mjs can resolve to .mts
.cjs can resolve to .cts

Closes PACK-3031

This is what tsc --traceResolution says about priority of the various possible resolution:

======== Resolving module '../libs/f.js' from '/Users/niklas/Desktop/nodenext-app/app/page.tsx'. ========
Explicitly specified module resolution kind: 'NodeNext'.
Resolving in CJS mode with conditions 'require', 'types', 'node'.
Loading module as file / folder, candidate module location '/Users/niklas/Desktop/nodenext-app/libs/f.js', target file types: TypeScript, JavaScript, Declaration.
File name '/Users/niklas/Desktop/nodenext-app/libs/f.js' has a '.js' extension - stripping it.
File '/Users/niklas/Desktop/nodenext-app/libs/f.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.tsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.d.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.jsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.tsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.d.ts' does not exist.

Testing Instructions

I also added a test case

Copy link

vercel bot commented Jul 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-gatsby-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-kitchensink-blog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-svelte-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
examples-vite-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2024 3:22pm

@turbo-orchestrator turbo-orchestrator bot added needs: triage New issues get this label. Remove it after triage owned-by: turbopack labels Jul 15, 2024
Copy link
Contributor

github-actions bot commented Jul 15, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

github-actions bot commented Jul 15, 2024

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Jul 15, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@mischnic mischnic force-pushed the mischnic/pack-3031-web-995-support-tsconfig-nodenext branch from 904e2f8 to 2e4ee59 Compare July 17, 2024 09:56
@turbo-orchestrator turbo-orchestrator bot added created-by: turbopack and removed needs: triage New issues get this label. Remove it after triage labels Jul 17, 2024
if options_value.enable_js_ts_rewriting {
let mut rewritten_path = path_pattern.clone();
let rewritten_path_modified =
rewritten_path.replace_final_constants(&|c: &RcStr| -> Option<Pattern> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should modify new_path

Should be outside of the if !options_value.fully_specified

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One sideeffect of that approach is that is also replaces the js that was appended via extensions:

request with appended extensions:

Alternatives([
    Constant("./src/foo.js"),
    Constant("./src/foo.js.ts"),
    Constant("./src/foo.js.js"),
    Constant("./src/foo.js.json")])

request with extensions and then the new replacement

Alternatives([
    Constant("./src/foo.ts"),
    Constant("./src/foo.tsx"),
    Constant("./src/foo.js"),
    Constant("./src/foo.js.ts"),  // <-
    Constant("./src/foo.js.ts"),  // <-
    Constant("./src/foo.js.tsx"), // <-
    Constant("./src/foo.js.js"),
    Constant("./src/foo.js.json")])

But in practice, it probably doesn't make a difference

@sokra sokra merged commit 64e5946 into main Jul 29, 2024
57 of 59 checks passed
@sokra sokra deleted the mischnic/pack-3031-web-995-support-tsconfig-nodenext branch July 29, 2024 14:53
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
### Description

Allow resolving for example `./foo.js` to `./foo.ts`

`.js` can resolve to `.ts` and `.tsx`
`.mjs` can resolve to `.mts`
`.cjs` can resolve to `.cts`

Closes PACK-3031

This is what `tsc --traceResolution` says about priority of the various
possible resolution:
```
======== Resolving module '../libs/f.js' from '/Users/niklas/Desktop/nodenext-app/app/page.tsx'. ========
Explicitly specified module resolution kind: 'NodeNext'.
Resolving in CJS mode with conditions 'require', 'types', 'node'.
Loading module as file / folder, candidate module location '/Users/niklas/Desktop/nodenext-app/libs/f.js', target file types: TypeScript, JavaScript, Declaration.
File name '/Users/niklas/Desktop/nodenext-app/libs/f.js' has a '.js' extension - stripping it.
File '/Users/niklas/Desktop/nodenext-app/libs/f.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.tsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.d.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.jsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.tsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.d.ts' does not exist.
```

### Testing Instructions

I also added a test case
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
### Description

Allow resolving for example `./foo.js` to `./foo.ts`

`.js` can resolve to `.ts` and `.tsx`
`.mjs` can resolve to `.mts`
`.cjs` can resolve to `.cts`

Closes PACK-3031

This is what `tsc --traceResolution` says about priority of the various
possible resolution:
```
======== Resolving module '../libs/f.js' from '/Users/niklas/Desktop/nodenext-app/app/page.tsx'. ========
Explicitly specified module resolution kind: 'NodeNext'.
Resolving in CJS mode with conditions 'require', 'types', 'node'.
Loading module as file / folder, candidate module location '/Users/niklas/Desktop/nodenext-app/libs/f.js', target file types: TypeScript, JavaScript, Declaration.
File name '/Users/niklas/Desktop/nodenext-app/libs/f.js' has a '.js' extension - stripping it.
File '/Users/niklas/Desktop/nodenext-app/libs/f.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.tsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.d.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.jsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.ts' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.tsx' does not exist.
File '/Users/niklas/Desktop/nodenext-app/libs/f.js.d.ts' does not exist.
```

### Testing Instructions

I also added a test case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants