这是indexloc提供的服务,不要输入任何密码
Skip to content

add support for dynamic requests in require() and import() #7153

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

Merged
merged 18 commits into from
Feb 14, 2024

Conversation

sokra
Copy link
Member

@sokra sokra commented Jan 29, 2024

Description

Refactors PatternMatching to support a map of multiple matches by key. Based on the new RequestKey support for resolving.

Code Generation for require("./dir/"+ expr) will emit a map of __turbopack_lookup__({ "./a": () => ..., "./b": () => ... }, "./dir/" + expr).

The output format isn't really optimized yet, that's work for future PRs.

Testing Instructions

added a test case, enabled one previously skipped test case

Closes PACK-986

Copy link

vercel bot commented Jan 29, 2024

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

Name Status Preview Comments Updated (UTC)
examples-svelte-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Feb 14, 2024 3:29pm
rust-docs ❌ Failed (Inspect) Feb 14, 2024 3:29pm
9 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview Feb 14, 2024 3:29pm

Copy link
Contributor

github-actions bot commented Jan 29, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Jan 29, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

@sokra sokra force-pushed the sokra/dynamic-request branch from a4b8e3b to 53f2bef Compare February 1, 2024 16:26
@sokra sokra force-pushed the sokra/dynamic-request branch from 53f2bef to dd4f59b Compare February 6, 2024 16:22
@sokra sokra force-pushed the sokra/dynamic-request branch from dd4f59b to f472811 Compare February 6, 2024 20:11
@sokra sokra force-pushed the sokra/dynamic-request branch from f472811 to 0be21e5 Compare February 9, 2024 14:12
@sokra sokra force-pushed the sokra/dynamic-request branch from 0be21e5 to 541fc3e Compare February 9, 2024 17:08
@sokra sokra marked this pull request as ready for review February 9, 2024 18:08
@sokra sokra requested a review from a team as a code owner February 9, 2024 18:08
@sokra sokra requested a review from ForsakenHarmony February 9, 2024 18:08
};
match args.into_iter().next() {
Some(ExprOrSpread { spread: None, expr: key_expr }) => {
*expr = pm.create_import(*key_expr, import_externals);
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if pm.create_import could return a CallExpr so we could still use the visit_mut_call_expr visitor

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically possible, but not sure if I want to put so much internal knowledge into the signature...

return Promise.resolve().then(() => {
throw e;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return Promise.resolve().then(() => {
throw e;
});
return Promise.reject(e);

Copy link
Member Author

Choose a reason for hiding this comment

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

That's different. Promise.reject causes an unhandled rejection, but promise resolve throw allows to register a catch before the promise is rejected

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be the case?

You can try running this in node.js:

const process = require('node:process');

process.on('unhandledRejection', (reason, promise) => {
  console.log('Unhandled Rejection at:', promise, 'reason:', reason);
});

function willReject() {
  return Promise.reject(new Error("rejected with error"))
}

async function willCatch() {
  try {
    await willReject()
  } catch(e) {
    console.error("caught in try catch", e)
  }
}

willReject().catch((err) => console.error("caught in handler", err))

willCatch()

* flags:
* * 1: Error as rejected promise
*/
function moduleLookup(
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can actually just combine this with requireContext?
(for import we could add another property)

just need to copy the generic error from here and maybe expand commonJsRequireContext in place instead of having it as a separate thing

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we should combine them. In a followup PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can do that, will change all the snapshots again I guess

@sokra sokra merged commit 5dbce38 into main Feb 14, 2024
@sokra sokra deleted the sokra/dynamic-request branch February 14, 2024 15:36
sokra added a commit that referenced this pull request Feb 15, 2024
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…rborepo#7153)

### Description

Refactors `PatternMatching` to support a map of multiple matches by key.
Based on the new `RequestKey` support for resolving.

Code Generation for `require("./dir/"+ expr)` will emit a map of
`__turbopack_lookup__({ "./a": () => ..., "./b": () => ... }, "./dir/" +
expr)`.

The output format isn't really optimized yet, that's work for future
PRs.

### Testing Instructions

added a test case, enabled one previously skipped test case


Closes PACK-986
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…rborepo#7153)

### Description

Refactors `PatternMatching` to support a map of multiple matches by key.
Based on the new `RequestKey` support for resolving.

Code Generation for `require("./dir/"+ expr)` will emit a map of
`__turbopack_lookup__({ "./a": () => ..., "./b": () => ... }, "./dir/" +
expr)`.

The output format isn't really optimized yet, that's work for future
PRs.

### Testing Instructions

added a test case, enabled one previously skipped test case


Closes PACK-986
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…rborepo#7153)

### Description

Refactors `PatternMatching` to support a map of multiple matches by key.
Based on the new `RequestKey` support for resolving.

Code Generation for `require("./dir/"+ expr)` will emit a map of
`__turbopack_lookup__({ "./a": () => ..., "./b": () => ... }, "./dir/" +
expr)`.

The output format isn't really optimized yet, that's work for future
PRs.

### Testing Instructions

added a test case, enabled one previously skipped test case


Closes PACK-986
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…rborepo#7153)

### Description

Refactors `PatternMatching` to support a map of multiple matches by key.
Based on the new `RequestKey` support for resolving.

Code Generation for `require("./dir/"+ expr)` will emit a map of
`__turbopack_lookup__({ "./a": () => ..., "./b": () => ... }, "./dir/" +
expr)`.

The output format isn't really optimized yet, that's work for future
PRs.

### Testing Instructions

added a test case, enabled one previously skipped test case


Closes PACK-986
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants