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

Turbopack + app router: always use externals for predefined packages #56440

Merged
merged 20 commits into from Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6c8e952
Add test for using external node_modules for pages
wbinnssmith Oct 2, 2023
7b13a60
fixup! Add test for using external node_modules for pages
wbinnssmith Oct 4, 2023
ab6e12f
fixup! Add test for using external node_modules for pages
wbinnssmith Oct 4, 2023
6a4acc7
fixup! Add test for using external node_modules for pages
wbinnssmith Oct 4, 2023
ec9ab85
Merge remote-tracking branch 'origin/canary' into wbinnssmith/pages-e…
wbinnssmith Oct 4, 2023
3485e0a
Turbopack + app router: always use externals for predefined packages
wbinnssmith Oct 4, 2023
60fd82a
fixup! Turbopack + app router: always use externals for predefined pa…
wbinnssmith Oct 4, 2023
5b0dbd2
fixup! Turbopack + app router: always use externals for predefined pa…
wbinnssmith Oct 5, 2023
c149b0c
fixup! Turbopack + app router: always use externals for predefined pa…
wbinnssmith Oct 5, 2023
cd83ef3
fixup! Turbopack + app router: always use externals for predefined pa…
wbinnssmith Oct 5, 2023
6a23aa6
fixup! Add test for using external node_modules for pages
wbinnssmith Oct 5, 2023
35b00e1
Merge remote-tracking branch 'origin/canary' into wbinnssmith/pages-e…
wbinnssmith Oct 5, 2023
6495fb5
Merge remote-tracking branch 'origin/wbinnssmith/pages-external' into…
wbinnssmith Oct 5, 2023
4f13a77
fixup! Turbopack + app router: always use externals for predefined pa…
wbinnssmith Oct 5, 2023
fb9878a
fixup! Turbopack + app router: always use externals for predefined pa…
wbinnssmith Oct 5, 2023
e8a27f6
fixup! Add test for using external node_modules for pages
wbinnssmith Oct 5, 2023
7f516f3
Merge remote-tracking branch 'origin/wbinnssmith/pages-external' into…
wbinnssmith Oct 5, 2023
f9cafbc
fixup! Turbopack + app router: always use externals for predefined pa…
wbinnssmith Oct 6, 2023
2ffc50c
Merge remote-tracking branch 'origin/canary' into wbinnssmith/app-ext…
wbinnssmith Oct 6, 2023
5380705
Merge branch 'canary' into wbinnssmith/app-external
kodiakhq[bot] Oct 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 17 additions & 2 deletions packages/next-swc/crates/next-core/src/next_server/context.rs
Expand Up @@ -64,7 +64,7 @@ use crate::{
get_decorators_transform_options, get_jsx_transform_options,
get_typescript_transform_options,
},
util::foreign_code_context_condition,
util::{foreign_code_context_condition, load_next_js_templateon},
};

#[turbo_tasks::value(serialization = "auto_for_input")]
Expand Down Expand Up @@ -105,10 +105,25 @@ pub async fn get_server_resolve_options_context(
let root_dir = project_path.root().resolve().await?;
let module_feature_report_resolve_plugin = ModuleFeatureReportResolvePlugin::new(project_path);
let unsupported_modules_resolve_plugin = UnsupportedModulesResolvePlugin::new(project_path);

// Always load these predefined packages as external.
let mut external_packages: Vec<String> = load_next_js_templateon(
project_path,
"dist/lib/server-external-packages.json".to_string(),
)
.await?;

// Add the config's own list of external packages.
external_packages.extend(
(*next_config.server_component_externals().await?)
.iter()
.cloned(),
);

let server_component_externals_plugin = ExternalCjsModulesResolvePlugin::new(
project_path,
project_path.root(),
ExternalPredicate::Only(next_config.server_component_externals()).cell(),
ExternalPredicate::Only(Vc::cell(external_packages)).cell(),
);
let ty = ty.into_value();

Expand Down
4 changes: 2 additions & 2 deletions packages/next-swc/crates/next-core/src/util.rs
Expand Up @@ -568,12 +568,12 @@ pub async fn load_next_js_templateon<T: DeserializeOwned>(
project_path: Vc<FileSystemPath>,
path: String,
) -> Result<T> {
let file_path = get_next_package(project_path).join(path);
let file_path = get_next_package(project_path).join(path.clone());

let content = &*file_path.read().await?;

let FileContent::Content(file) = content else {
bail!("Expected file content for metrics data");
bail!("Expected file content at {}", path);
};

let result: T = parse_json_rope_with_source_context(file.content())?;
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/app-dir/externals/app/layout.tsx
@@ -0,0 +1,7 @@
export default function Root({ children }: { children: React.ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/externals/app/page.tsx
@@ -0,0 +1,5 @@
import { foo } from 'external-package'

export default function Page() {
return <div>{foo}</div>
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/externals/app/predefined/page.tsx
@@ -0,0 +1,5 @@
import { foo } from 'sqlite3'

export default function Predefined() {
return <div>{foo}</div>
}
71 changes: 71 additions & 0 deletions test/e2e/app-dir/externals/externals.test.ts
@@ -0,0 +1,71 @@
import fs from 'fs/promises'
import path from 'path'
import { createNextDescribe } from 'e2e-utils'

async function getAppPageChunkPaths(appDir: string, pageName?: string) {
const rscPath = path.join(appDir, '.next/server/chunks/rsc')
const pageRegex = new RegExp(
`app${pageName ? '_' + pageName : ''}_page_tsx_[0-9a-f]+._.js$`
)

return (await fs.readdir(rscPath))
.filter((p) => p.match(pageRegex))
.map((basename) => path.join(rscPath, basename))
}

createNextDescribe(
'externals-app',
{
files: __dirname,
},
({ next, isNextDev, isTurbopack }) => {
it('should have externals for those in config.experimental.serverComponentsExternalPackages', async () => {
await next.render$('/')

if (isTurbopack) {
const appBundles = await getAppPageChunkPaths(next.testDir)
const bundleTexts = await Promise.all(
appBundles.map((b) => fs.readFile(b, 'utf8'))
)
expect(
bundleTexts.find((t) =>
t.includes(
'__turbopack_external_require__("external-package", true)'
)
)
).not.toBeUndefined()
} else {
const output = await fs.readFile(
path.join(next.testDir, '.next/server/app/page.js'),
'utf8'
)
expect(output).toContain('require("external-package")')
}
})

it('uses externals for predefined list in server-external-packages.json', async () => {
await next.render$('/predefined')

if (isTurbopack) {
const appBundles = await getAppPageChunkPaths(
next.testDir,
'predefined'
)
const bundleTexts = await Promise.all(
appBundles.map((b) => fs.readFile(b, 'utf8'))
)
expect(
bundleTexts.find((t) =>
t.includes('__turbopack_external_require__("sqlite3", true)')
)
).not.toBeUndefined()
} else {
const output = await fs.readFile(
path.join(next.testDir, '.next/server/app/predefined/page.js'),
'utf8'
)
expect(output).toContain('require("sqlite3")')
}
})
}
)
10 changes: 10 additions & 0 deletions test/e2e/app-dir/externals/next.config.js
@@ -0,0 +1,10 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {
experimental: {
serverComponentsExternalPackages: ['external-package'],
},
}

module.exports = nextConfig

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/e2e/app-dir/externals/node_modules/sqlite3/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.