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

feat(next-core): apply invalid import assertion on the remaining contexts #63146

Merged
merged 3 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/next-swc/crates/next-core/src/next_client/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ use crate::{
},
next_shared::{
resolve::{
ModuleFeatureReportResolvePlugin, NextSharedRuntimeResolvePlugin,
UnsupportedModulesResolvePlugin,
get_invalid_server_only_resolve_plugin, ModuleFeatureReportResolvePlugin,
NextSharedRuntimeResolvePlugin, UnsupportedModulesResolvePlugin,
},
transforms::{
emotion::get_emotion_transform_rule, relay::get_relay_transform_rule,
Expand Down Expand Up @@ -162,6 +162,7 @@ pub async fn get_client_resolve_options_context(
Vc::upcast(ModuleFeatureReportResolvePlugin::new(project_path)),
Vc::upcast(UnsupportedModulesResolvePlugin::new(project_path)),
Vc::upcast(NextSharedRuntimeResolvePlugin::new(project_path)),
Vc::upcast(get_invalid_server_only_resolve_plugin(project_path)),
],
..Default::default()
};
Expand Down
11 changes: 7 additions & 4 deletions packages/next-swc/crates/next-core/src/next_server/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ use crate::{
next_server::resolve::ExternalPredicate,
next_shared::{
resolve::{
get_invalid_client_only_resolve_plugin, ModuleFeatureReportResolvePlugin,
NextExternalResolvePlugin, NextNodeSharedRuntimeResolvePlugin,
UnsupportedModulesResolvePlugin,
get_invalid_client_only_resolve_plugin, get_invalid_styled_jsx_resolve_plugin,
ModuleFeatureReportResolvePlugin, NextExternalResolvePlugin,
NextNodeSharedRuntimeResolvePlugin, UnsupportedModulesResolvePlugin,
},
transforms::{
emotion::get_emotion_transform_rule, get_ecma_transform_rule,
Expand Down Expand Up @@ -114,6 +114,8 @@ pub async fn get_server_resolve_options_context(
let module_feature_report_resolve_plugin = ModuleFeatureReportResolvePlugin::new(project_path);
let unsupported_modules_resolve_plugin = UnsupportedModulesResolvePlugin::new(project_path);
let invalid_client_only_resolve_plugin = get_invalid_client_only_resolve_plugin(project_path);
let invalid_styled_jsx_client_only_resolve_plugin =
get_invalid_styled_jsx_resolve_plugin(project_path);

// Always load these predefined packages as external.
let mut external_packages: Vec<String> = load_next_js_templateon(
Expand Down Expand Up @@ -221,6 +223,7 @@ pub async fn get_server_resolve_options_context(
| ServerContextType::AppRoute { .. }
| ServerContextType::Instrumentation => {
plugins.push(Vc::upcast(invalid_client_only_resolve_plugin));
plugins.push(Vc::upcast(invalid_styled_jsx_client_only_resolve_plugin));
}
ServerContextType::AppSSR { .. } => {
//[TODO] Build error in this context makes rsc-build-error.ts fail which expects runtime error code
Expand Down Expand Up @@ -548,7 +551,7 @@ pub async fn get_server_module_options_context(
..
} => {
let mut custom_source_transform_rules: Vec<ModuleRule> =
vec![styled_components_transform_rule]
vec![styled_components_transform_rule, styled_jsx_transform_rule]
.into_iter()
.flatten()
.collect();
Expand Down
38 changes: 30 additions & 8 deletions packages/next-swc/crates/next-core/src/next_shared/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ impl ResolvePlugin for UnsupportedModulesResolvePlugin {
pub struct InvalidImportModuleIssue {
pub file_path: Vc<FileSystemPath>,
pub messages: Vec<String>,
pub skip_context_message: bool,
}

#[turbo_tasks::value_impl]
Expand Down Expand Up @@ -141,19 +142,20 @@ impl Issue for InvalidImportModuleIssue {
let raw_context = &*self.file_path.await?;

let mut messages = self.messages.clone();
messages.push("\n".to_string());

//[TODO]: how do we get the import trace?
messages.push(format!(
"The error was caused by importing '{}'",
raw_context.path
));
if !self.skip_context_message {
//[TODO]: how do we get the import trace?
messages.push(format!(
"The error was caused by importing '{}'",
raw_context.path
));
}

Ok(Vc::cell(Some(
StyledString::Line(
messages
.iter()
.map(|v| StyledString::Text(v.into()))
.map(|v| StyledString::Text(format!("{}\n", v)))
.collect::<Vec<StyledString>>(),
)
.cell(),
Expand Down Expand Up @@ -205,6 +207,8 @@ impl ResolvePlugin for InvalidImportResolvePlugin {
InvalidImportModuleIssue {
file_path: context,
messages: self.message.clone(),
// styled-jsx specific resolve error have own message
skip_context_message: self.invalid_import == "styled-jsx",
}
.cell()
.emit();
Expand Down Expand Up @@ -235,7 +239,6 @@ pub(crate) fn get_invalid_client_only_resolve_plugin(
/// Returns a resolve plugin if context have imports to `server-only`.
/// Only the contexts that alises `server-only` to
/// `next/dist/compiled/server-only/index` should use this.
#[allow(unused)]
pub(crate) fn get_invalid_server_only_resolve_plugin(
root: Vc<FileSystemPath>,
) -> Vc<InvalidImportResolvePlugin> {
Expand All @@ -250,6 +253,25 @@ pub(crate) fn get_invalid_server_only_resolve_plugin(
)
}

/// Returns a resolve plugin if context have imports to `styled-jsx`.
pub(crate) fn get_invalid_styled_jsx_resolve_plugin(
root: Vc<FileSystemPath>,
) -> Vc<InvalidImportResolvePlugin> {
InvalidImportResolvePlugin::new(
root,
"styled-jsx".to_string(),
vec![
"'client-only' cannot be imported from a Server Component module. It should only be \
used from a Client Component."
.to_string(),
"The error was caused by using 'styled-jsx'. It only works in a Client Component but \
none of its parents are marked with \"use client\", so they're Server Components by \
default."
.to_string(),
],
)
}

#[turbo_tasks::value]
pub(crate) struct NextExternalResolvePlugin {
root: Vc<FileSystemPath>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use turbopack_binding::turbopack::{
use super::get_ecma_transform_rule;
use crate::next_config::NextConfig;

/// Returns a transform rule for the relay graphql transform.
/// Returns a transform rule for the styled jsx transform.
pub async fn get_styled_jsx_transform_rule(
next_config: Vc<NextConfig>,
target_browsers: Vc<RuntimeVersions>,
Expand Down
27 changes: 18 additions & 9 deletions test/development/acceptance-app/invalid-imports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,26 @@ describe('Error Overlay invalid imports', () => {
await session.patch(pageFile, withoutUseClient)

expect(await session.hasRedbox()).toBe(true)
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"./app/comp2.js
'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.
if (process.env.TURBOPACK) {
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"./app
Invalid import
'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.
The error was caused by using 'styled-jsx'. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default."
`)
} else {
expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
"./app/comp2.js
'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.

The error was caused by using 'styled-jsx' in './app/comp2.js'. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.
The error was caused by using 'styled-jsx' in './app/comp2.js'. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.

Import trace for requested module:
./app/comp2.js
./app/comp1.js
./app/page.js"
`)
Import trace for requested module:
./app/comp2.js
./app/comp1.js
./app/page.js"
`)
}

await cleanup()
})
Expand Down
5 changes: 2 additions & 3 deletions test/turbopack-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -1070,12 +1070,11 @@
},
"test/development/acceptance-app/invalid-imports.test.ts": {
"passed": [
"Error Overlay invalid imports should show error when external package imports client-only in server component"
],
"failed": [
"Error Overlay invalid imports should show error when external package imports client-only in server component",
"Error Overlay invalid imports should show error when external package imports server-only in client component",
"Error Overlay invalid imports should show error when using styled-jsx in server component"
],
"failed": [],
"pending": [],
"flakey": [],
"runtimeError": false
Expand Down