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.

Sign up for GitHub

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

Jump to bottom

major: output ESM for .mjs or "type": "module" builds #720

Merged
merged 24 commits into from Jul 16, 2021
Merged

major: output ESM for .mjs or "type": "module" builds #720

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1ccdd50
Revert "increase jest watcher timeout"
guybedford Jul 5, 2021
3808d04
--esm flag for emitting es modules
guybedford Jun 26, 2021
595b92f
add fixture
guybedford Jun 26, 2021
42098d2
disable v8cache for esm
guybedford Jul 3, 2021
adba554
use node14 target
guybedford Jul 3, 2021
bfdbbc4
output esm automatically for mjs / type module
guybedford Jul 5, 2021
62128ff
mjs for mjs
guybedford Jul 5, 2021
33d17dc
update unit tests
guybedford Jul 5, 2021
696cb75
update cli tests
guybedford Jul 5, 2021
cfa7e2f
update asset relocator loader
guybedford Jul 7, 2021
b095409
fixup filename handling
guybedford Jul 7, 2021
0a86107
fixup resolve ref
guybedford Jul 7, 2021
cb1a8a3
fixup fs ref
guybedford Jul 7, 2021
baaa12c
update unit tests
guybedford Jul 7, 2021
ccd57ba
fixup esm check
guybedford Jul 7, 2021
369ac30
fixup external type
guybedford Jul 7, 2021
1ee927e
fixup webpack externals
guybedford Jul 7, 2021
4758297
webpack@5.44
guybedford Jul 10, 2021
8c557aa
update fixtures
guybedford Jul 10, 2021
d71b185
add fixtures update script
guybedford Jul 10, 2021
c6d63a2
unify package boundary check
guybedford Jul 12, 2021
c33e2ac
Remove newline
styfle Jul 12, 2021
c01d291
fixup: pr feedback
guybedford Jul 13, 2021
981e853
fixup: include has type module module
guybedford Jul 13, 2021
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
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Expand Up @@ -28,7 +28,7 @@
"@sentry/node": "^4.3.0",
"@slack/web-api": "^5.13.0",
"@tensorflow/tfjs-node": "^0.3.0",
"@vercel/webpack-asset-relocator-loader": "1.5.0",
"@vercel/webpack-asset-relocator-loader": "1.6.0",
"analytics-node": "^3.3.0",
"apollo-server-express": "^2.2.2",
"arg": "^4.1.0",
Expand Down Expand Up @@ -110,7 +110,7 @@
"vue": "^2.5.17",
"vue-server-renderer": "^2.5.17",
"web-vitals": "^0.2.4",
"webpack": "5.43.0",
"webpack": "5.44.0",
"when": "^3.7.8"
},
"resolutions": {
Expand Down
2 changes: 2 additions & 0 deletions readme.md
View file
Open in desktop
Expand Up @@ -38,6 +38,8 @@ Eg:
$ ncc build input.js -o dist
```

If building an `.mjs` or `.js` module inside a `"type": "module"` [package boundary](https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#packages_package_json_and_file_extensions), an ES module output will be created automatically.

Outputs the Node.js compact build of `input.js` into `dist/index.js`.

> Note: If the input file is using a `.cjs` extension, then so will the corresponding output file.
Expand Down
21 changes: 19 additions & 2 deletions src/cli.js
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@ const glob = require("glob");
const shebangRegEx = require("./utils/shebang");
const rimraf = require("rimraf");
const crypto = require("crypto");
const { writeFileSync, unlink, existsSync, symlinkSync } = require("fs");
const { writeFileSync, unlink, existsSync, symlinkSync, readFileSync } = require("fs");
const mkdirp = require("mkdirp");
const { version: nccVersion } = require('../package.json');

Expand All @@ -24,6 +24,7 @@ Commands:

Options:
-o, --out [file] Output directory for build (defaults to dist)

-m, --minify Minify output
styfle marked this conversation as resolved.
Show resolved Hide resolved
-C, --no-cache Skip build cache population
-s, --source-map Generate source map
Expand Down Expand Up @@ -218,17 +219,33 @@ async function runCmd (argv, stdout, stderr) {
);
if (existsSync(outDir))
rimraf.sync(outDir);
mkdirp.sync(outDir);
run = true;

// fallthrough
case "build":
if (args._.length > 2)
errTooManyArguments("build");

function hasTypeModule (path) {
let root = resolve('/');
while ((path = resolve(path, '..')) !== root) {
guybedford marked this conversation as resolved.
Show resolved Hide resolved
try {
return JSON.parse(readFileSync(eval('resolve')(path, 'package.json')).toString()).type === 'module';
}
catch (e) {
if (e.code === 'ENOENT')
continue;
styfle marked this conversation as resolved.
Show resolved Hide resolved
throw e;
}
}
}

let startTime = Date.now();
let ps;
const buildFile = eval("require.resolve")(resolve(args._[1] || "."));
const ext = buildFile.endsWith('.cjs') ? '.cjs' : '.js';
const esm = buildFile.endsWith('.mjs') || !buildFile.endsWith('.cjs') && hasTypeModule(buildFile);
const ext = buildFile.endsWith('.cjs') ? '.cjs' : esm && (buildFile.endsWith('.mjs') || !hasTypeModule(buildFile)) ? '.mjs' : '.js';
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why you didn't decide to preserve the extension of the input file?

I would expect this:

.cjs => .cjs
.mjs => .mjs
.js => .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.

@Style we do keep the extension so that .cjs -> .cjs and .mjs -> .mjs. The issue with .js is that it will be read by webpack as supporting both CJS and ESM, so isn't enough information on its own, hence the boundary check.

const ncc = require("./index.js")(
buildFile,
{
Expand Down
61 changes: 47 additions & 14 deletions src/index.js
View file
Open in desktop
Expand Up @@ -30,14 +30,29 @@ const defaultPermissions = 0o666;

const relocateLoader = eval('require(__dirname + "/loaders/relocate-loader.js")');

function hasTypeModule (path) {
guybedford marked this conversation as resolved.
Show resolved Hide resolved
let root = pathResolve('/');
while ((path = pathResolve(path, '..')) !== root) {
try {
return JSON.parse(fs.readFileSync(eval('pathResolve')(path, 'package.json')).toString()).type === 'module';
}
catch (e) {
if (e.code === 'ENOENT')
continue;
throw e;
}
}
}

module.exports = ncc;
function ncc (
entry,
{
cache,
customEmit = undefined,
esm = entry.endsWith('.mjs') || !entry.endsWith('.cjs') && hasTypeModule(entry),
externals = [],
filename = 'index' + (entry.endsWith('.cjs') ? '.cjs' : '.js'),
filename = 'index' + (!esm && entry.endsWith('.cjs') ? '.cjs' : esm && (entry.endsWith('.mjs') || !hasTypeModule(entry)) ? '.mjs' : '.js'),
minify = false,
sourceMap = false,
sourceMapRegister = true,
Expand All @@ -55,6 +70,10 @@ function ncc (
production = true,
} = {}
) {
// v8 cache not supported for ES modules
if (esm)
v8cache = false;
guybedford marked this conversation as resolved.
Show resolved Hide resolved

const cjsDeps = () => ({
mainFields: ["main"],
extensions: SUPPORTED_EXTENSIONS,
Expand All @@ -74,7 +93,7 @@ function ncc (

if (!quiet) {
console.log(`ncc: Version ${nccVersion}`);
console.log(`ncc: Compiling file ${filename}`);
console.log(`ncc: Compiling file ${filename} into ${esm ? 'ESM' : 'CJS'}`);
}

if (target && !target.startsWith('es')) {
Expand Down Expand Up @@ -233,7 +252,8 @@ function ncc (
},
amd: false,
experiments: {
topLevelAwait: true
topLevelAwait: true,
outputModule: esm
},
optimization: {
nodeEnv: false,
Expand All @@ -247,7 +267,7 @@ function ncc (
},
devtool: sourceMap ? "cheap-module-source-map" : false,
mode: "production",
target: target ? ["node", target] : "node",
target: target ? ["node14", target] : "node14",
stats: {
logging: 'error'
},
Expand All @@ -258,8 +278,9 @@ function ncc (
path: "/",
// Webpack only emits sourcemaps for files ending in .js
filename: ext === '.cjs' ? filename + '.js' : filename,
libraryTarget: "commonjs2",
strictModuleExceptionHandling: true
libraryTarget: esm ? 'module' : 'commonjs2',
strictModuleExceptionHandling: true,
module: esm
},
resolve: {
extensions: SUPPORTED_EXTENSIONS,
Expand All @@ -286,9 +307,9 @@ function ncc (
},
// https://github.com/vercel/ncc/pull/29#pullrequestreview-177152175
node: false,
externals ({ context, request }, callback) {
externals ({ context, request, dependencyType }, callback) {
const external = externalMap.get(request);
if (external) return callback(null, `commonjs ${external}`);
if (external) return callback(null, `${dependencyType === 'esm' && esm ? 'module' : 'node-commonjs'} ${external}`);
return callback();
},
module: {
Expand Down Expand Up @@ -465,12 +486,13 @@ function ncc (
let result;
try {
result = await terser.minify(code, {
module: esm,
compress: false,
mangle: {
keep_classnames: true,
keep_fnames: true
},
sourceMap: sourceMap ? {
sourceMap: map ? {
content: map,
filename,
url: `${filename}.map`
Expand All @@ -483,10 +505,10 @@ function ncc (

({ code, map } = {
code: result.code,
map: sourceMap ? JSON.parse(result.map) : undefined
map: map ? JSON.parse(result.map) : undefined
guybedford marked this conversation as resolved.
Show resolved Hide resolved
});
}
catch {
catch (e) {
console.log('An error occurred while minifying. The result will not be minified.');
}
}
Expand All @@ -511,9 +533,20 @@ function ncc (
`if (cachedData) process.on('exit', () => { try { writeFileSync(basename + '.cache', script.createCachedData()); } catch(e) {} });\n`;
}

if (sourceMap && sourceMapRegister) {
code = `require('./sourcemap-register${ext}');` + code;
assets[`sourcemap-register${ext}`] = { source: fs.readFileSync(`${__dirname}/sourcemap-register.js.cache.js`), permissions: defaultPermissions };
if (map && sourceMapRegister) {
const registerExt = esm ? '.cjs' : ext;
code = (esm ? `import './sourcemap-register${registerExt}';` : `require('./sourcemap-register${registerExt}');`) + code;
assets[`sourcemap-register${registerExt}`] = { source: fs.readFileSync(`${__dirname}/sourcemap-register.js.cache.js`), permissions: defaultPermissions };
}

if (esm && !filename.endsWith('.mjs')) {
// always output a "type": "module" package JSON for esm builds
const baseDir = dirname(filename);
const pjsonPath = (baseDir === '.' ? '' : baseDir) + 'package.json';
if (assets[pjsonPath])
assets[pjsonPath].source = JSON.stringify(Object.assign(JSON.parse(pjsonPath.source.toString()), { type: 'module' }));
else
assets[pjsonPath] = { source: JSON.stringify({ type: 'module' }, null, 2) + '\n', permissions: defaultPermissions };
}

if (shebangMatch) {
Expand Down
9 changes: 8 additions & 1 deletion test/cli.js
View file
Open in desktop
Expand Up @@ -73,7 +73,7 @@
{
args: ["build", "-o", "tmp", "test/fixtures/test.mjs"],
expect (code, stdout, stderr) {
return stdout.toString().indexOf('tmp/index.js') !== -1;
return stdout.toString().indexOf('tmp/index.mjs') !== -1;
}
},
{
Expand Down Expand Up @@ -103,5 +103,12 @@
expect (code, stdout) {
return code === 0 && stdout.indexOf('ncc built-in') !== -1;
},
},
{
args: ["build", "-o", "tmp", "test/fixtures/module.cjs"],
expect (code, stdout) {
const fs = require('fs');
return code === 0 && fs.readFileSync('tmp/index.js', 'utf8').toString().indexOf('export {') === -1;
}
}
]
3 changes: 3 additions & 0 deletions test/fixtures/module.cjs
View file
Open in desktop
@@ -0,0 +1,3 @@
require('./no-dep.js');

exports.aRealExport = true;
4 changes: 3 additions & 1 deletion test/unit/bundle-subasset/output-coverage.js
View file
Open in desktop
Expand Up @@ -84,7 +84,9 @@ module.exports = require("path");
/******/
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";/************************************************************************/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";
/******/
/************************************************************************/
var __webpack_exports__ = {};
// This entry need to be wrapped in an IIFE because it need to be isolated against other modules in the chunk.
(() => {
Expand Down
4 changes: 3 additions & 1 deletion test/unit/bundle-subasset/output.js
View file
Open in desktop
Expand Up @@ -84,7 +84,9 @@ module.exports = require("path");
/******/
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";/************************************************************************/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";
/******/
/************************************************************************/
var __webpack_exports__ = {};
// This entry need to be wrapped in an IIFE because it need to be isolated against other modules in the chunk.
(() => {
Expand Down
4 changes: 3 additions & 1 deletion test/unit/bundle-subasset2/output-coverage.js
View file
Open in desktop
Expand Up @@ -75,7 +75,9 @@
/******/
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";/************************************************************************/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";
/******/
/************************************************************************/
var __webpack_exports__ = {};
// ESM COMPAT FLAG
__nccwpck_require__.r(__webpack_exports__);
Expand Down
4 changes: 3 additions & 1 deletion test/unit/bundle-subasset2/output.js
View file
Open in desktop
Expand Up @@ -75,7 +75,9 @@
/******/
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";/************************************************************************/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";
/******/
/************************************************************************/
var __webpack_exports__ = {};
// ESM COMPAT FLAG
__nccwpck_require__.r(__webpack_exports__);
Expand Down
4 changes: 3 additions & 1 deletion test/unit/custom-emit/output-coverage.js
View file
Open in desktop
Expand Up @@ -44,7 +44,9 @@ module.exports = require("fs");
/************************************************************************/
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";/************************************************************************/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";
/******/
/************************************************************************/
var __webpack_exports__ = {};
// This entry need to be wrapped in an IIFE because it need to be isolated against other modules in the chunk.
(() => {
Expand Down
4 changes: 3 additions & 1 deletion test/unit/custom-emit/output.js
View file
Open in desktop
Expand Up @@ -44,7 +44,9 @@ module.exports = require("fs");
/************************************************************************/
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";/************************************************************************/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";
/******/
/************************************************************************/
var __webpack_exports__ = {};
// This entry need to be wrapped in an IIFE because it need to be isolated against other modules in the chunk.
(() => {
Expand Down
32 changes: 6 additions & 26 deletions test/unit/exports-nomodule/output-coverage.js
View file
Open in desktop
@@ -1,34 +1,14 @@
/******/ (() => { // webpackBootstrap
/******/ "use strict";
/******/ // The require scope
/******/ var __nccwpck_require__ = {};
/******/
/******/ "use strict";
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = new URL('.', import.meta.url).pathname.slice(import.meta.url.match(/^file:\/\/\/\w:/) ? 1 : 0, -1) + "/";
/******/
/************************************************************************/
/******/ /* webpack/runtime/make namespace object */
/******/ (() => {
/******/ // define __esModule on exports
/******/ __nccwpck_require__.r = (exports) => {
/******/ if(typeof Symbol !== 'undefined' && Symbol.toStringTag) {
/******/ Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
/******/ }
/******/ Object.defineProperty(exports, '__esModule', { value: true });
/******/ };
/******/ })();
/******/
/******/ /* webpack/runtime/compat */
/******/
/******/ if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";/************************************************************************/
var __webpack_exports__ = {};
// ESM COMPAT FLAG
__nccwpck_require__.r(__webpack_exports__);

;// CONCATENATED MODULE: ./test/unit/exports-nomodule/node.js
var x = 'x';

;// CONCATENATED MODULE: ./test/unit/exports-nomodule/input.js

console.log(x);

module.exports = __webpack_exports__;
/******/ })()
;
console.log(x);