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

fix: apply type: module only to runtime modules #2549

Merged
merged 4 commits into from
Jul 22, 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
4 changes: 2 additions & 2 deletions src/build/content/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,9 @@ export const copyNextDependencies = async (ctx: PluginContext): Promise<void> =>
await tracer.withActiveSpan('copyNextDependencies', async () => {
const entries = await readdir(ctx.standaloneDir)
const promises: Promise<void>[] = entries.map(async (entry) => {
// copy all except the package.json and distDir (.next) folder as this is handled in a separate function
// copy all except the distDir (.next) folder as this is handled in a separate function
// this will include the node_modules folder as well
if (entry === 'package.json' || entry === ctx.nextDistDir) {
if (entry === ctx.nextDistDir) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are no longer writing top level package.json (we write it just for runtime modules, so it no longer is top-level), hence this copies top-level package.json provided by Next.js (which is really just site's package.json) so it will "inherit" type from user's site

return
}
const src = join(ctx.standaloneDir, entry)
Expand Down
21 changes: 11 additions & 10 deletions src/build/functions/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,20 @@ const copyHandlerDependencies = async (ctx: PluginContext) => {
)
}

// We need to create a package.json file with type: module to make sure that the runtime modules
// are handled correctly as ESM modules
promises.push(
writeFile(
join(ctx.serverHandlerRuntimeModulesDir, 'package.json'),
JSON.stringify({ type: 'module' }),
),
)

const fileList = await glob('dist/**/*', { cwd: ctx.pluginDir })

for (const filePath of fileList) {
promises.push(
cp(join(ctx.pluginDir, filePath), join(ctx.serverHandlerDir, '.netlify', filePath), {
cp(join(ctx.pluginDir, filePath), join(ctx.serverHandlerRuntimeModulesDir, filePath), {
recursive: true,
force: true,
}),
Expand Down Expand Up @@ -85,13 +94,6 @@ const writeHandlerManifest = async (ctx: PluginContext) => {
)
}

const writePackageMetadata = async (ctx: PluginContext) => {
await writeFile(
join(ctx.serverHandlerRootDir, 'package.json'),
JSON.stringify({ type: 'module' }),
)
}

const applyTemplateVariables = (template: string, variables: Record<string, string>) => {
return Object.entries(variables).reduce((acc, [key, value]) => {
return acc.replaceAll(key, value)
Expand Down Expand Up @@ -136,13 +138,12 @@ export const clearStaleServerHandlers = async (ctx: PluginContext) => {
*/
export const createServerHandler = async (ctx: PluginContext) => {
await tracer.withActiveSpan('createServerHandler', async () => {
await mkdir(join(ctx.serverHandlerDir, '.netlify'), { recursive: true })
await mkdir(join(ctx.serverHandlerRuntimeModulesDir), { recursive: true })

await copyNextServerCode(ctx)
await copyNextDependencies(ctx)
await copyHandlerDependencies(ctx)
await writeHandlerManifest(ctx)
await writePackageMetadata(ctx)
await writeHandlerFile(ctx)

await verifyHandlerDirStructure(ctx)
Expand Down
4 changes: 4 additions & 0 deletions src/build/plugin-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ export class PluginContext {
return join(this.serverHandlerRootDir, this.distDirParent)
}

get serverHandlerRuntimeModulesDir(): string {
return join(this.serverHandlerDir, '.netlify')
}

get nextServerHandler(): string {
if (this.relativeAppDir.length !== 0) {
return join(this.lambdaWorkingDirectory, '.netlify/dist/run/handlers/server.js')
Expand Down
11 changes: 11 additions & 0 deletions tests/e2e/simple-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,14 @@ test('Compressed rewrites are readable', async ({ simple }) => {
expect(resp.headers.get('content-encoding')).toEqual('br')
expect(await resp.text()).toContain('<title>Example Domain</title>')
})

test('can require CJS module that is not bundled', async ({ simple }) => {
const resp = await fetch(`${simple.url}/api/cjs-file-with-js-extension`)

expect(resp.status).toBe(200)

const parsedBody = await resp.json()

expect(parsedBody.notBundledCJSModule.isBundled).toEqual(false)
expect(parsedBody.bundledCJSModule.isBundled).toEqual(true)
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const { parse: pathParse } = require('node:path')

const fileBase = pathParse(__filename).base

module.exports = {
fileBase,
// if fileBase is not the same as this module name, it was bundled
isBundled: fileBase !== 'bundled.cjs',
}
11 changes: 11 additions & 0 deletions tests/fixtures/simple/app/api/cjs-file-with-js-extension/route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { NextResponse } from 'next/server'
import { resolve } from 'node:path'

export async function GET() {
return NextResponse.json({
notBundledCJSModule: __non_webpack_require__(resolve('./cjs-file-with-js-extension.js')),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

__non_webpack_require__ turns into regular require in compiled output (instead of inlining modules by webpack) which is needed to reproduce the ERR_REQUIRE_ESM problem (without usage of 3rd party packages)

bundledCJSModule: require('./bundled.cjs'),
})
}

export const dynamic = 'force-dynamic'
9 changes: 9 additions & 0 deletions tests/fixtures/simple/cjs-file-with-js-extension.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const { parse: pathParse } = require('node:path')

const fileBase = pathParse(__filename).base

module.exports = {
fileBase,
// if fileBase is not the same as this module name, it was bundled
isBundled: fileBase !== 'cjs-file-with-js-extension.js',
}
14 changes: 14 additions & 0 deletions tests/integration/simple-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,20 @@ test.skipIf(process.env.NEXT_VERSION !== 'canary')<FixtureTestContext>(
},
)

test<FixtureTestContext>('can require CJS module that is not bundled', async (ctx) => {
await createFixture('simple', ctx)
await runPlugin(ctx)

const response = await invokeFunction(ctx, { url: '/api/cjs-file-with-js-extension' })

expect(response.statusCode).toBe(200)

const parsedBody = JSON.parse(response.body)

expect(parsedBody.notBundledCJSModule.isBundled).toEqual(false)
expect(parsedBody.bundledCJSModule.isBundled).toEqual(true)
})

describe('next patching', async () => {
const { cp: originalCp, appendFile } = (await vi.importActual(
'node:fs/promises',
Expand Down
Loading