Skip to content

Commit

Permalink
Generate "type": "module" npm packages when ESM is enabled
Browse files Browse the repository at this point in the history 
See sass/dart-sass#1995
  • Loading branch information
@nex3
nex3 committed Jun 8, 2023
1 parent f0b618e commit 66b7dba
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 33 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 2.4.6

* Properly mark NPM packages as `"type": "module"` when `pkg.jsEsmExports` is
set, and mark all CJS files as explicitly `.cjs`.

## 2.4.5

* Properly set the pub credentials for the latest versions of the Dart SDK.
Expand Down
70 changes: 44 additions & 26 deletions lib/src/npm.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,22 @@ final npmDistTag = InternalConfigVariable.fn<String>(() {
return firstComponent is String ? firstComponent : "pre";
});

/// Whether we're generating a package that supports ESM imports.
bool get _supportsEsm => jsEsmExports.value != null;

/// The file extension for CommonJS files in the generated NPM package.
///
/// If the NPM package supports ESM, we treat that as canonical and add an
/// explicit extension for CJS files. Otherwise, we treat CJS as canonical.
String get _cjs => _supportsEsm ? '.cjs' : '.js';

/// The file extension for ESM files in the generated NPM package.
///
/// This is always `.js`, because we treat ESM as canonical if it's being
/// generated at all. We still store it as a variable to document that the files
/// are expected to be ESM.
const _mjs = '.js';

/// Whether [addNpmTasks] has been called yet.
var _addedNpmTasks = false;

Expand Down Expand Up @@ -206,7 +222,7 @@ void addNpmTasks() {
if (hasNonCliRequires) {
fail("If jsModuleMainLibrary isn't set, all jsRequires must have "
"JSRequireTarget.cli or JSRequireTarget.all.");
} else if (jsEsmExports.value != null) {
} else if (_supportsEsm) {
fail("If jsEsmExports is set, jsModuleMainLibrary must be set as well.");
}
}
Expand Down Expand Up @@ -350,7 +366,7 @@ Future<void> _buildPackage() async {
dir.createSync(recursive: true);

var extractedRequires = _copyJSAndInjectDependencies(
'build/$_npmName.dart.js', p.join(dir.path, '$_npmName.dart.js'));
'build/$_npmName.dart.js', p.join(dir.path, '$_npmName.dart$_cjs'));
var allRequires =
_requiresForTarget(JSRequireTarget.all).union(extractedRequires);

Expand All @@ -364,14 +380,14 @@ Future<void> _buildPackage() async {
jsonEncode({
...npmPackageJson.value,
"version": version.toString(),
"bin": {for (var name in executables.value.keys) name: "$name.js"},
if (_supportsEsm) "type": "module",
"bin": {for (var name in executables.value.keys) name: "$name$_cjs"},
if (jsModuleMainLibrary.value != null)
"main": "$_npmName.${nodeRequires.isEmpty ? 'default' : 'node'}"
".${jsEsmExports.value == null ? 'js' : 'cjs'}",
"main": "$_npmName.${nodeRequires.isEmpty ? 'default' : 'node'}$_cjs",
if (npmPackageJson.value["exports"] is Map ||
nodeRequires.isNotEmpty ||
browserRequires.isNotEmpty ||
jsEsmExports.value != null)
_supportsEsm)
"exports": {
if (npmPackageJson.value["exports"] is Map)
...npmPackageJson.value["exports"] as Map,
Expand All @@ -389,33 +405,33 @@ Future<void> _buildPackage() async {
""");

if (jsEsmExports.value != null) {
if (_supportsEsm) {
buffer.writeln("""
require('./$_npmName.dart.js');
require('./$_npmName.dart$_cjs');
var library = globalThis._cliPkgExports.pop();
if (globalThis._cliPkgExports.length === 0) delete globalThis._cliPkgExports;
""");
} else {
buffer.writeln("var library = require('./$_npmName.dart.js');");
buffer.writeln("var library = require('./$_npmName.dart$_cjs');");
}

buffer.writeln(_loadRequires(cliRequires.union(allRequires)));
buffer.writeln(
"library.${_executableIdentifiers[name]}(process.argv.slice(2));");
writeString(p.join('build', 'npm', '$name.js'), buffer.toString());
writeString(p.join('build', 'npm', '$name$_cjs'), buffer.toString());
}

if (jsModuleMainLibrary.value != null) {
if (nodeRequires.isNotEmpty) {
_writePlatformWrapper(p.join('build', 'npm', '$_npmName.node.js'),
_writePlatformWrapper(p.join('build', 'npm', '$_npmName.node'),
nodeRequires.union(allRequires));
}
if (browserRequires.isNotEmpty) {
_writePlatformWrapper(p.join('build', 'npm', '$_npmName.browser.js'),
_writePlatformWrapper(p.join('build', 'npm', '$_npmName.browser'),
browserRequires.union(allRequires));
}

_writePlatformWrapper(p.join('build', 'npm', '$_npmName.default.js'),
_writePlatformWrapper(p.join('build', 'npm', '$_npmName.default'),
defaultRequires.union(allRequires));
}

Expand Down Expand Up @@ -467,7 +483,7 @@ JSRequireSet _copyJSAndInjectDependencies(String source, String destination) {
var buffer = StringBuffer();

var exportsVariable = "exports";
if (jsEsmExports.value != null) {
if (_supportsEsm) {
buffer.writeln("""
// Because of vitejs/vite#12340, there's no way to reliably detect whether we're
// running as a (possibly bundled/polyfilled) ESM module or as a CommonJS
Expand Down Expand Up @@ -522,21 +538,23 @@ JSRequireSet _requiresForTarget(JSRequireTarget target) =>

/// Returns a single string specifier for `package.exports` if [jsEsmExports]
/// isn't set, or a conditional export if it is.
Object _exportSpecifier(String name) => jsEsmExports.value == null
? "./$_npmName.$name.js"
: {"require": "./$_npmName.$name.cjs", "default": "./$_npmName.$name.js"};
Object _exportSpecifier(String name) => _supportsEsm
? {"require": "./$_npmName.$name$_cjs", "default": "./$_npmName.$name$_mjs"}
: "./$_npmName.$name$_cjs";

/// Writes one or two wrappers that loads and re-exports `$_npmName.dart.js`
/// Writes one or two wrappers that loads and re-exports `$_npmName.dart.[c]js`
/// with [requires] injected.
///
/// The [requires] should not have the final `.[cm]js` extension.
///
/// This writes both an ESM and a CJS wrapper if [jsEsmExports] is set.
void _writePlatformWrapper(String path, JSRequireSet requires) {
var exports = jsEsmExports.value;
if (exports != null) {
_writeImportWrapper(path, requires, exports);
_writeRequireWrapper(p.setExtension(path, '.cjs'), requires);
_writeImportWrapper('$path$_mjs', requires, exports);
_writeRequireWrapper('$path$_cjs', requires);
} else {
_writeRequireWrapper(path, requires);
_writeRequireWrapper('$path$_cjs', requires);
}
}

Expand All @@ -545,12 +563,12 @@ void _writePlatformWrapper(String path, JSRequireSet requires) {
void _writeRequireWrapper(String path, JSRequireSet requires) {
writeString(
path,
(jsEsmExports.value == null
? "const library = require('./$_npmName.dart.js');\n"
: "require('./$_npmName.dart.js');\n"
(_supportsEsm
? "require('./$_npmName.dart$_cjs');\n"
"const library = globalThis._cliPkgExports.pop();\n"
"if (globalThis._cliPkgExports.length === 0) delete "
"globalThis._cliPkgExports;\n") +
"globalThis._cliPkgExports;\n"
: "const library = require('./$_npmName.dart$_cjs');\n") +
"${_loadRequires(requires)}\n"
"module.exports = library;\n");
}
Expand Down Expand Up @@ -581,7 +599,7 @@ void _writeImportWrapper(

buffer
..write("""
import ${json.encode('./$_npmName.dart.js')};
import ${json.encode('./$_npmName.dart$_cjs')};
const _cliPkgLibrary = globalThis._cliPkgExports.pop();
if (globalThis._cliPkgExports.length === 0) delete globalThis._cliPkgExports;
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cli_pkg
version: 2.4.5
version: 2.4.6
description: Grinder tasks for releasing Dart CLI packages.
homepage: https://github.com/google/dart_cli_pkg

Expand Down
46 changes: 40 additions & 6 deletions test/npm_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ void main() {
await (await grind(["pkg-npm-dev"])).shouldExit();

var process = await TestProcess.start(
"node$dotExe", [d.path("my_app/build/npm/exec.js")]);
"node$dotExe", [d.path("my_app/build/npm/exec.cjs")]);
expect(process.stdout, emitsInOrder(["Hello from exec", emitsDone]));
await process.shouldExit(0);
});
Expand Down Expand Up @@ -650,22 +650,22 @@ void main() {
.validate();
});

test("generates ESM files if jsEsmExports is set", () async {
test("generates loadable ESM files if jsEsmExports is set", () async {
await d.package(pubspec, """
void main(List<String> args) {
pkg.jsModuleMainLibrary.value = "lib/src/module_main.dart";
pkg.jsModuleMainLibrary.value = "lib/src/exports.dart";
pkg.jsRequires.value = [
pkg.JSRequire('util', target: pkg.JSRequireTarget.cli),
pkg.JSRequire('other', target: pkg.JSRequireTarget.node),
pkg.JSRequire('os', target: pkg.JSRequireTarget.node),
];
pkg.jsEsmExports.value = {};
pkg.jsEsmExports.value = {'hello'};
pkg.addNpmTasks();
grind(args);
}
""", [
_packageJson,
d.dir("lib/src", [d.file("module_main.dart", "void main() {}")])
d.dir("lib/src", [_exportsHello('osLoaded')])
]).create();
await (await grind(["pkg-npm-dev"])).shouldExit();

Expand All @@ -685,6 +685,40 @@ void main() {
}
})))
.validate();

await d.dir("depender", [
d.file(
"package.json",
json.encode({
"dependencies": {"my_app": "file:../my_app/build/npm"}
})),
d.file("test.mjs", """
import * as myApp from "my_app";
console.log(myApp.hello);
"""),
d.file("test.cjs", """
const myApp = require("my_app");
console.log(myApp.hello);
""")
]).create();

await (await TestProcess.start("npm", ["install"],
runInShell: true, workingDirectory: d.path("depender")))
.shouldExit(0);

var mjsProcess =
await TestProcess.start("node$dotExe", [d.path("depender/test.mjs")]);
expect(mjsProcess.stdout, emits("true"));
;
await mjsProcess.shouldExit(0);

var cjsProcess =
await TestProcess.start("node$dotExe", [d.path("depender/test.cjs")]);
expect(cjsProcess.stdout, emits("true"));
;
await cjsProcess.shouldExit(0);
});

test("overwrite existing string value", () async {
Expand Down

0 comments on commit 66b7dba

Please sign in to comment.