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 13 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion packages/next-swc/crates/next-core/src/next_import_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
local::{NextFontLocalCssModuleReplacer, NextFontLocalReplacer},
},
next_server::context::ServerContextType,
util::NextRuntime,
util::{load_next_js_templateon, NextRuntime},
};

// Make sure to not add any external requests here.
Expand Down Expand Up @@ -292,6 +292,18 @@ pub async fn get_next_server_import_map(
"react-server-dom-webpack/",
ImportMapping::External(Some("react-server-dom-turbopack".into())).cell(),
);

// Always load these predefined packages as external.
let external_packages: Vec<String> = load_next_js_templateon(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this just be done with ExternalCjsModulesResolvePlugin?

project_path,
"dist/lib/server-external-packages.json".to_string(),
)
.await?;

for external_package in external_packages {
import_map.insert_exact_alias(&external_package, external);
import_map.insert_wildcard_alias(external_package + "/", external);
}
}
ServerContextType::Middleware => {}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/next-swc/crates/next-core/src/util.rs
Original file line number Diff line number Diff line change
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
12 changes: 12 additions & 0 deletions test/integration/externals-app-bundle/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const metadata = {
title: 'Next.js',
description: 'Generated by Next.js',
}

export default function RootLayout({ children }) {
return (
<html lang="en">
<body>{children}</body>
</html>
)
}
5 changes: 5 additions & 0 deletions test/integration/externals-app-bundle/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { foo } from 'external-package'

export default function Index() {
return <div>{foo}</div>
}
5 changes: 5 additions & 0 deletions test/integration/externals-app-bundle/app/predefined/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { foo } from 'sqlite3'

export default function Predefined() {
return <div>{foo}</div>
}
5 changes: 5 additions & 0 deletions test/integration/externals-app-bundle/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
experimental: {
serverComponentsExternalPackages: ['external-package'],
},
}

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.

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.

80 changes: 80 additions & 0 deletions test/integration/externals-app-bundle/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/* eslint-env jest */

import fs from 'fs/promises'
import { join } from 'path'
import {
killApp,
launchApp,
findPort,
renderViaHTTP,
waitFor,
} from 'next-test-utils'

const appDir = join(__dirname, '../')

describe('app bundle externals ', () => {
describe('dev mode', () => {
it('should have externals for those in config.experimental.serverComponentsExternalPackages', async () => {
const port = await findPort()
const app = await launchApp(appDir, port)
await renderViaHTTP(port, '/')
await waitFor(1000)
if (process.env.TURBOPACK) {
const appBundles = await getAppPageChunkPaths(appDir)
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(
join(appDir, '.next/server/app/page.js'),
'utf8'
)
expect(output).toContain('require("external-package")')
}
await killApp(app)
})

it('uses externals for predefined list in server-external-packages.json', async () => {
const port = await findPort()
const app = await launchApp(appDir, port)
await renderViaHTTP(port, '/predefined')
await waitFor(1000)
if (process.env.TURBOPACK) {
const appBundles = await getAppPageChunkPaths(appDir, '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(
join(appDir, '.next/server/app/predefined/page.js'),
'utf8'
)
expect(output).toContain('require("sqlite3")')
}
await killApp(app)
})
})
})

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

return (await fs.readdir(rscPath))
.filter((p) => p.match(pageRegex))
.map((basename) => join(rscPath, basename))
}
49 changes: 49 additions & 0 deletions test/integration/externals-pages-bundle/test/externals.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/* eslint-env jest */

import fs from 'fs/promises'
import { join } from 'path'
import {
killApp,
launchApp,
findPort,
File,
renderViaHTTP,
waitFor,
} from 'next-test-utils'

const appDir = join(__dirname, '../')

describe('default', () => {
it('should use externals for unvendored node_modules reachable from the project', async () => {
const port = await findPort()
const config = new File(join(appDir, 'next.config.js'))
config.delete()
try {
const app = await launchApp(appDir, port)
await renderViaHTTP(port, '/')
if (process.env.TURBOPACK) {
const ssrPath = join(appDir, '.next/server/chunks/ssr')
const pageBundleBasename = (await fs.readdir(ssrPath)).find((p) =>
p.match(/pages_index_[0-9a-f]+\.js$/)
)
expect(pageBundleBasename).not.toBeUndefined()
const output = await fs.readFile(
join(ssrPath, pageBundleBasename),
'utf8'
)
expect(output).toContain(
'__turbopack_external_require__("external-package", true)'
)
} else {
const output = await fs.readFile(
join(appDir, '.next/server/pages/index.js'),
'utf8'
)
expect(output).toContain('require("external-package")')
}
await killApp(app)
} finally {
config.restore()
}
})
})
Loading