Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

ESM support: allow usage in type:"module" packages #1800

Merged
merged 5 commits into from
Sep 30, 2021
Merged

Conversation

benmccann
Copy link
Member

Inspired by discussion with @Conduitry in sveltejs/svelte#6772 (comment)

The goal of this PR would be to ease the migration path from Sapper to SvelteKit by allowing developers to convert their project to ESM as a first step independent of the rest of the migration to SvelteKit

@benmccann benmccann changed the title ESM support: allow usage in type:"module" packages ESM support: allow usage in type:"module" packages Sep 30, 2021
Copy link
Member

@Conduitry Conduitry left a comment

Choose a reason for hiding this comment

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

I would suggest the following changes, lightly tested. It stores the 'is module' boolean in the env module with a couple of other things, and then uses this value both in cli.ts where we write __sapper__/build/index.js and in config/rollup.ts where we determine the Rollup output config.

diff --git a/src/cli.ts b/src/cli.ts
index ef3b00c..93d4c7d 100755
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -5,6 +5,7 @@ import colors from 'kleur';
 import * as pkg from '../package.json';
 import { elapsed, repeat, left_pad, format_milliseconds } from './utils';
 import { InvalidEvent, ErrorEvent, FatalEvent, BuildEvent, ReadyEvent } from './interfaces';
+import { module } from './config/env';
 
 const prog = sade('sapper').version(pkg.version);
 
@@ -172,8 +173,7 @@ prog.command('build [dest]')
 			await _build(opts.bundler, opts.legacy, opts.cwd, opts.src, opts.routes, opts.output, dest, opts.ext);
 
 			const launcher = path.resolve(dest, 'index.js');
-			const pkg = await import(path.resolve(opts.cwd, 'package.json'));
-			const import_statement = pkg.type === 'module'
+			const import_statement = module
 				? "import './server/server.js';"
 				: "require('./server/server.js');";
 
diff --git a/src/config/env.ts b/src/config/env.ts
index 87cf4ae..a708421 100644
--- a/src/config/env.ts
+++ b/src/config/env.ts
@@ -1,7 +1,9 @@
 export let dev: boolean;
 export let src: string;
 export let dest: string;
+export let module: boolean;
 
 export const set_dev = (_: boolean) => dev = _;
 export const set_src = (_: string) => src = _;
 export const set_dest = (_: string) => dest = _;
+export const set_module = (_: boolean) => module = _;
diff --git a/src/config/rollup.ts b/src/config/rollup.ts
index d41b3c5..9e5e057 100644
--- a/src/config/rollup.ts
+++ b/src/config/rollup.ts
@@ -1,4 +1,4 @@
-import { dev, src, dest } from './env';
+import { dev, src, dest, module } from './env';
 import { InputOption, OutputOptions } from 'rollup';
 
 const sourcemap = dev ? 'inline' : false;
@@ -32,11 +32,10 @@ export default {
 			};
 		},
 
-		output: (opts): OutputOptions => {
-			const { format } = opts || {};
+		output: (): OutputOptions => {
 			return {
 				dir: `${dest}/server`,
-				format: format || 'cjs',
+				format: module ? 'esm' : 'cjs',
 				sourcemap
 			};
 		}
diff --git a/src/core/create_compilers/index.ts b/src/core/create_compilers/index.ts
index ecc741f..3775984 100644
--- a/src/core/create_compilers/index.ts
+++ b/src/core/create_compilers/index.ts
@@ -1,7 +1,8 @@
+import * as fs from 'fs';
 import * as path from 'path';
 import RollupCompiler from './RollupCompiler';
 import { WebpackCompiler } from './WebpackCompiler';
-import { set_dev, set_src, set_dest } from '../../config/env';
+import { set_dev, set_src, set_dest, set_module } from '../../config/env';
 
 export type Compiler = RollupCompiler | WebpackCompiler;
 
@@ -22,6 +23,7 @@ export default async function create_compilers(
 	set_dev(dev);
 	set_src(src);
 	set_dest(dest);
+	set_module(JSON.parse(fs.readFileSync(path.resolve(cwd, 'package.json'), 'utf-8')).type === 'module');
 
 	if (bundler === 'rollup') {
 		const config = await RollupCompiler.load_config(cwd);

@benmccann
Copy link
Member Author

Thanks for the suggestions. I like those ideas and have updated it to match

@Conduitry
Copy link
Member

Hmm, all the tests failing look like they're because lots of them don't have their own individual package.jsons. It would probably be easiest (and a little bit safer) to make the compiler just assume CJS if it doesn't see a package.json.

@Conduitry Conduitry merged commit bf4858c into master Sep 30, 2021
@Conduitry Conduitry deleted the esm-support branch September 30, 2021 22:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants