-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Turbopack: Add BeforeResolve
plugin type
#8165
Conversation
This adds a new plugin type for replacing resolution _before_ it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy. This is used to replace `ImportMapping` replacers for `@next/font/local` in an upcoming Next.js PR
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
🟢 Turbopack Benchmark CI successful 🟢Thanks |
✅ This change can build |
@@ -72,6 +72,7 @@ pub struct ResolveOptionsContext { | |||
/// A list of plugins which get applied before (in the future) and after |
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.
probably need to adjust comment bit, also we may want to rename plugins
in this case?
… show custom error message Depends on vercel/turborepo#8165 This: - Creates and uses a `BeforeResolvePlugin` to handle requests to `next/font/local/target.css` instead of `ImportMapping` replacers - Returns a `ResolveResultItem::Error` which includes a custom `StyledString` describing the missing font file Test Plan: `TURBOPACK=1 pnpm test-dev test/e2e/app-dir/next-font/next-font.test.ts`
|
) -> Result<Option<Vc<ResolveResult>>> { | ||
for plugin in &options.await?.before_resolve_plugins { | ||
if let Some(result) = *plugin | ||
.before_resolve(lookup_path, reference_type.clone(), request) |
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.
That might have a negative performance effect, since it calls the plugin for every resolving in the application. So something like with the resolve plugins would be beneficial: before_resolve_condition
@@ -66,6 +67,7 @@ pub enum ModuleResolveResultItem { | |||
OutputAsset(Vc<Box<dyn OutputAsset>>), | |||
External(String, ExternalType), | |||
Ignore, | |||
Error(Vc<StyledString>), |
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.
Error(Vc<StyledString>), | |
RuntimeError(Vc<String>), |
I was thinking for this to lead to a runtime error during code generation.
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.
Shouldn't this be a build error -- we wouldn't want to produce something that is runnable if this is the case. I think that's what happens with Webpack.
@@ -2553,7 +2619,18 @@ pub async fn handle_resolve_error( | |||
.cell() | |||
.emit(); | |||
} | |||
result | |||
|
|||
if let Some(error_string) = result.await?.get_first_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.
I think it should not do that. It should just return the result even if that's a resolve result with runtime error.
The issue should be emitted earlier. E.g. in the before resolve plugin itself
… show custom error message (#65870) Depends on vercel/turborepo#8165 This: - Creates and uses a `BeforeResolvePlugin` to handle requests to `next/font/local/target.css` instead of `ImportMapping` replacers - Returns a `ResolveResultItem::Error` which includes a custom `StyledString` describing the missing font file Test Plan: `TURBOPACK=1 pnpm test-dev test/e2e/app-dir/next-font/next-font.test.ts`
This adds a new plugin type for replacing resolution _before_ it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy. This is used to replace `ImportMapping` replacers for `@next/font/local` in an upcoming Next.js PR
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
This adds a new plugin type for replacing resolution _before_ it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy. This is used to replace `ImportMapping` replacers for `@next/font/local` in an upcoming Next.js PR
This adds a new plugin type for replacing resolution _before_ it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy. This is used to replace `ImportMapping` replacers for `@next/font/local` in an upcoming Next.js PR
This adds a new plugin type for replacing resolution _before_ it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy. This is used to replace `ImportMapping` replacers for `@next/font/local` in an upcoming Next.js PR
This adds a new plugin type for replacing resolution _before_ it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy. This is used to replace `ImportMapping` replacers for `@next/font/local` in an upcoming Next.js PR
This adds a new plugin type for replacing resolution _before_ it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy. This is used to replace `ImportMapping` replacers for `@next/font/local` in an upcoming Next.js PR
… show custom error message (#65870) Depends on vercel/turborepo#8165 This: - Creates and uses a `BeforeResolvePlugin` to handle requests to `next/font/local/target.css` instead of `ImportMapping` replacers - Returns a `ResolveResultItem::Error` which includes a custom `StyledString` describing the missing font file Test Plan: `TURBOPACK=1 pnpm test-dev test/e2e/app-dir/next-font/next-font.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
… show custom error message (#65870) Depends on vercel/turborepo#8165 This: - Creates and uses a `BeforeResolvePlugin` to handle requests to `next/font/local/target.css` instead of `ImportMapping` replacers - Returns a `ResolveResultItem::Error` which includes a custom `StyledString` describing the missing font file Test Plan: `TURBOPACK=1 pnpm test-dev test/e2e/app-dir/next-font/next-font.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
… show custom error message (#65870) Depends on vercel/turborepo#8165 This: - Creates and uses a `BeforeResolvePlugin` to handle requests to `next/font/local/target.css` instead of `ImportMapping` replacers - Returns a `ResolveResultItem::Error` which includes a custom `StyledString` describing the missing font file Test Plan: `TURBOPACK=1 pnpm test-dev test/e2e/app-dir/next-font/next-font.test.ts`
vercel/turborepo#8165 introduced plugins that operate before resolving occurs, meaning that plugins like `InvalidImportResolvePlugin` which never use the initial resolve result and report issues can avoid that work. Test Plan: `TURBOPACK_DEV=1 TURBOPACK=1 pnpm test-dev test/development/acceptance-app/invalid-imports.test.ts`
This adds a new plugin type for replacing resolution before it occurs. This allows plugin authors to intercept requests for modules in place of the typical resolution strategy.
This is used to replace
ImportMapping
replacers for@next/font/local
in an upcoming Next.js PR