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

bundle with rollup instead of esbuild #6896

Merged
merged 5 commits into from Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/odd-beds-smile.md
@@ -0,0 +1,5 @@
---
'@sveltejs/adapter-node': patch
---

Bundle with rollup instead of esbuild
26 changes: 17 additions & 9 deletions packages/adapter-node/index.js
@@ -1,6 +1,10 @@
import { readFileSync, writeFileSync } from 'fs';
import { fileURLToPath } from 'url';
import * as esbuild from 'esbuild';
import { rollup } from 'rollup';
import { nodeResolve } from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import json from '@rollup/plugin-json';
Copy link
Member

Choose a reason for hiding this comment

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

do we need the JSON plugin? will people be importing JSON that hasn't already been bundled by Vite?

Copy link
Member

Choose a reason for hiding this comment

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

They could be using a devdependency that includes JSON which isn't bundled yet but which we want to bundle when adapting.

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, this needs to work with require('./stuff.json')


const files = fileURLToPath(new URL('./files', import.meta.url).href);

Expand Down Expand Up @@ -49,16 +53,20 @@ export default function (opts = {}) {

const pkg = JSON.parse(readFileSync('package.json', 'utf8'));

await esbuild.build({
platform: 'node',
sourcemap: 'linked',
target: 'es2022',
entryPoints: [`${tmp}/index.js`, `${tmp}/manifest.js`],
outdir: `${out}/server`,
splitting: true,
const bundle = await rollup({
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
input: {
index: `${tmp}/index.js`,
manifest: `${tmp}/manifest.js`
},
external: [...Object.keys(pkg.dependencies || {})],
plugins: [nodeResolve(), commonjs(), json()]
});

await bundle.write({
dir: `${out}/server`,
format: 'esm',
bundle: true,
external: [...Object.keys(pkg.dependencies || {})]
sourcemap: true,
chunkFileNames: `chunks/[name]-[hash].js`
});

builder.copy(files, out, {
Expand Down
3 changes: 2 additions & 1 deletion packages/adapter-node/package.json
Expand Up @@ -45,6 +45,7 @@
"uvu": "^0.5.3"
},
"dependencies": {
"esbuild": "^0.15.7"
"@rollup/plugin-commonjs": "^22.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

do we want these two as dependencies and @rollup/plugin-json in devDependencies? I'd expect them to be in the same place

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, fixed

"@rollup/plugin-node-resolve": "^14.1.0"
}
}
39 changes: 6 additions & 33 deletions pnpm-lock.yaml

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