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

Warn when implicitly using default export mode #3659

Merged
merged 4 commits into from Jul 18, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/run/batchWarnings.ts
Expand Up @@ -137,7 +137,7 @@ const deferredHandlers: {

MIXED_EXPORTS: warnings => {
title('Mixing named and default exports');
info(`https://rollupjs.org/guide/en/#output-exports`);
info(`https://rollupjs.org/guide/en/#outputexports`);
stderr(bold('The following entry modules are using named and default exports together:'));
const displayedWarnings = warnings.length > 5 ? warnings.slice(0, 3) : warnings;
for (const warning of displayedWarnings) {
Expand Down
42 changes: 35 additions & 7 deletions docs/999-big-list-of-options.md
Expand Up @@ -922,29 +922,57 @@ Default: `'auto'`

What export mode to use. Defaults to `auto`, which guesses your intentions based on what the `input` module exports:

* `default` – suitable if you are only exporting one thing using `export default ...`
* `named` – suitable if you are exporting more than one thing
* `none` – suitable if you are not exporting anything (e.g. you are building an app, not a library)
* `default` – if you are only exporting one thing using `export default ...`; note that this can cause issues when generating CommonJS output that is meant to be interchangeable with ESM output, see below
* `named` – if you are using named exports
* `none` – if you are not exporting anything (e.g. you are building an app, not a library)

As this is only an output transformation, you can only choose `default` if there a default export is the only export for all entry chunks. Likewise, you can only choose `none` if there are no exports, otherwise Rollup will error.

The difference between `default` and `named` affects how other people can consume your bundle. If you use `default`, a CommonJS user could do this, for example:

```js
const yourLib = require( 'your-lib' );
// your-lib package entry
export default "Hello world";

// a CommonJS consumer
/* require( "your-lib" ) returns "Hello World" */
const hello = require( "your-lib" );
```

With `named`, a user would do this instead:

```js
const yourMethod = require( 'your-lib' ).yourMethod;
// your-lib package entry
export const hello = "Hello world";

// a CommonJS consumer
/* require( "your-lib" ) returns {hello: "Hello World"} */
const hello = require( "your-lib" ).hello;
/* or using destructuring */
const {hello} = require( "your-lib" );
```

The wrinkle is that if you use `named` exports but *also* have a `default` export, a user would have to do something like this to use the default export:

```js
const yourMethod = require( 'your-lib' ).yourMethod;
const yourLib = require( 'your-lib' ).default;
// your-lib package entry
export default "foo";
export const bar = "bar";

// a CommonJS consumer
/* require( "your-lib" ) returns {default: "foo", bar: "bar"} */
const foo = require( "your-lib" ).default;
const bar = require( "your-lib" ).bar;
/* or using destructuring */
const {default: foo, bar} = require( "your-lib" );
```

Note: There are some tools such as Babel, TypeScript, Webpack, and `@rollup/plugin-commonjs` that are capable of resolving a CommonJS `require(...)` call with an ES module. If you are generating CommonJS output that is meant to be interchangeable with ESM output for those tools, you should always use `named` export mode. The reason is that most of those tools will by default return the namespace of an ES module on `require` where the default export is the `.default` property.

In other words for those tools, you cannot create a package interface where `const lib = require("your-lib")` yields the same as `import lib from "your-lib"`. With named export mode however, `const {lib} = require("your-lib")` will be equivalent to `import {lib} from "your-lib"`.

To alert you to this, Rollup will generate a warning when you encounter such a situation and did not select an explicit value for `output.exports`.

#### output.externalLiveBindings
Type: `boolean`<br>
CLI: `--externalLiveBindings`/`--no-externalLiveBindings`<br>
Expand Down
2 changes: 2 additions & 0 deletions rollup.config.js
Expand Up @@ -111,6 +111,8 @@ export default command => {
chunkFileNames: 'shared/[name].js',
dir: 'dist',
entryFileNames: '[name]',
// TODO Only loadConfigFile is using default exports mode; this should be changed in Rollup@3
exports: 'auto',
externalLiveBindings: false,
format: 'cjs',
freeze: false,
Expand Down
1 change: 1 addition & 0 deletions src/Chunk.ts
Expand Up @@ -295,6 +295,7 @@ export default class Chunk {
this.exportMode = getExportMode(
this,
this.outputOptions,
this.unsetOptions,
this.facadeModule!.id,
this.inputOptions.onwarn
);
Expand Down
15 changes: 13 additions & 2 deletions src/utils/error.ts
Expand Up @@ -58,6 +58,7 @@ export enum Errors {
MIXED_EXPORTS = 'MIXED_EXPORTS',
NAMESPACE_CONFLICT = 'NAMESPACE_CONFLICT',
PLUGIN_ERROR = 'PLUGIN_ERROR',
PREFER_NAMED_EXPORTS = 'PREFER_NAMED_EXPORTS',
UNRESOLVED_ENTRY = 'UNRESOLVED_ENTRY',
UNRESOLVED_IMPORT = 'UNRESOLVED_IMPORT',
VALIDATION_ERROR = 'VALIDATION_ERROR',
Expand Down Expand Up @@ -161,7 +162,7 @@ export function errInvalidExportOptionValue(optionValue: string) {
return {
code: Errors.INVALID_EXPORT_OPTION,
message: `"output.exports" must be "default", "named", "none", "auto", or left unspecified (defaults to "auto"), received "${optionValue}"`,
url: `https://rollupjs.org/guide/en/#output-exports`
url: `https://rollupjs.org/guide/en/#outputexports`
};
}

Expand Down Expand Up @@ -261,7 +262,7 @@ export function errMixedExport(facadeModuleId: string, name?: string) {
)}" is using named and default exports together. Consumers of your bundle will have to use \`${
name || 'chunk'
}["default"]\` to access the default export, which may not be what you want. Use \`output.exports: "named"\` to disable this warning`,
url: `https://rollupjs.org/guide/en/#output-exports`
url: `https://rollupjs.org/guide/en/#outputexports`
};
}

Expand All @@ -283,6 +284,16 @@ export function errNamespaceConflict(
};
}

export function errPreferNamedExports(facadeModuleId: string) {
const file = relativeId(facadeModuleId);
return {
code: Errors.PREFER_NAMED_EXPORTS,
id: facadeModuleId,
message: `Entry module "${file}" is implicitly using "default" export mode, which means for CommonJS output that its default export is assigned to "module.exports". For many tools, such CommonJS output will not be interchangeable with the original ES module. If this is intended, explicitly set "output.exports" to either "auto" or "default", otherwise you might want to consider changing the signature of "${file}" to use named exports only.`,
url: `https://rollupjs.org/guide/en/#outputexports`
};
}

export function errEntryCannotBeExternal(unresolvedId: string) {
return {
code: Errors.UNRESOLVED_ENTRY,
Expand Down
11 changes: 10 additions & 1 deletion src/utils/getExportMode.ts
@@ -1,10 +1,16 @@
import Chunk from '../Chunk';
import { NormalizedOutputOptions, WarningHandler } from '../rollup/types';
import { errIncompatibleExportOptionValue, errMixedExport, error } from './error';
import {
errIncompatibleExportOptionValue,
errMixedExport,
error,
errPreferNamedExports
} from './error';

export default function getExportMode(
chunk: Chunk,
{ exports: exportMode, name, format }: NormalizedOutputOptions,
unsetOptions: Set<string>,
facadeModuleId: string,
warn: WarningHandler
) {
Expand All @@ -22,6 +28,9 @@ export default function getExportMode(
if (exportKeys.length === 0) {
exportMode = 'none';
} else if (exportKeys.length === 1 && exportKeys[0] === 'default') {
if (format === 'cjs' && unsetOptions.has('exports')) {
warn(errPreferNamedExports(facadeModuleId));
}
exportMode = 'default';
} else {
if (format !== 'es' && exportKeys.indexOf('default') !== -1) {
Expand Down
13 changes: 9 additions & 4 deletions src/utils/options/normalizeOutputOptions.ts
Expand Up @@ -39,7 +39,7 @@ export function normalizeOutputOptions(
dynamicImportFunction: getDynamicImportFunction(config, inputOptions),
entryFileNames: getEntryFileNames(config, unsetOptions),
esModule: (config.esModule as boolean | undefined) ?? true,
exports: getExports(config),
exports: getExports(config, unsetOptions),
extend: (config.extend as boolean | undefined) || false,
externalLiveBindings: (config.externalLiveBindings as boolean | undefined) ?? true,
file,
Expand Down Expand Up @@ -120,7 +120,7 @@ const getFormat = (config: GenericConfigObject): InternalModuleFormat => {
default:
return error({
message: `You must specify "output.format", which can be one of "amd", "cjs", "system", "es", "iife" or "umd".`,
url: `https://rollupjs.org/guide/en/#output-format`
url: `https://rollupjs.org/guide/en/#outputformat`
});
}
};
Expand Down Expand Up @@ -223,9 +223,14 @@ const getEntryFileNames = (config: GenericConfigObject, unsetOptions: Set<string
return configEntryFileNames ?? '[name].js';
};

function getExports(config: GenericConfigObject): 'default' | 'named' | 'none' | 'auto' {
function getExports(
config: GenericConfigObject,
unsetOptions: Set<string>
): 'default' | 'named' | 'none' | 'auto' {
const configExports = config.exports as string | undefined;
if (configExports && !['default', 'named', 'none', 'auto'].includes(configExports)) {
if (configExports == null) {
unsetOptions.add('exports');
} else if (!['default', 'named', 'none', 'auto'].includes(configExports)) {
return error(errInvalidExportOptionValue(configExports));
}
return (configExports as 'default' | 'named' | 'none' | 'auto') || 'auto';
Expand Down
74 changes: 35 additions & 39 deletions test/chunking-form/index.js
Expand Up @@ -8,14 +8,14 @@ runTestSuiteWithSamples('chunking form', path.resolve(__dirname, 'samples'), (di
(config.skip ? describe.skip : config.solo ? describe.only : describe)(
path.basename(dir) + ': ' + config.description,
() => {
let rollupPromise;
let bundle;

FORMATS.forEach(format =>
it('generates ' + format, () => {
for (const format of FORMATS) {
it('generates ' + format, async () => {
process.chdir(dir);
return (
rollupPromise ||
(rollupPromise = rollup.rollup(
bundle =
bundle ||
(await rollup.rollup(
Object.assign(
{
input: [dir + '/main.js'],
Expand All @@ -36,42 +36,38 @@ runTestSuiteWithSamples('chunking form', path.resolve(__dirname, 'samples'), (di
},
config.options || {}
)
))
).then(bundle =>
generateAndTestBundle(
bundle,
Object.assign(
{
dir: dir + '/_actual/' + format,
format,
chunkFileNames: 'generated-[name].js'
},
(config.options || {}).output || {}
),
dir + '/_expected/' + format,
config
)
));
await generateAndTestBundle(
bundle,
Object.assign(
{
dir: dir + '/_actual/' + format,
exports: 'auto',
format,
chunkFileNames: 'generated-[name].js'
},
(config.options || {}).output || {}
),
dir + '/_expected/' + format,
config
);
})
);
});
}
}
);
});

function generateAndTestBundle(bundle, outputOptions, expectedDir, config) {
return bundle
.write(outputOptions)
.then(() => {
if (outputOptions.format === 'amd' && config.runAmd) {
return new Promise(resolve => {
global.assert = require('assert');
const requirejs = require('requirejs');
requirejs.config({ baseUrl: outputOptions.dir });
requirejs(['main'], main => {
Promise.resolve(config.runAmd.exports && config.runAmd.exports(main)).then(resolve);
});
});
}
})
.then(() => assertDirectoriesAreEqual(outputOptions.dir, expectedDir));
async function generateAndTestBundle(bundle, outputOptions, expectedDir, config) {
await bundle.write(outputOptions);
if (outputOptions.format === 'amd' && config.runAmd) {
await new Promise(resolve => {
global.assert = require('assert');
const requirejs = require('requirejs');
requirejs.config({ baseUrl: outputOptions.dir });
requirejs(['main'], main => {
Promise.resolve(config.runAmd.exports && config.runAmd.exports(main)).then(resolve);
});
});
}
assertDirectoriesAreEqual(outputOptions.dir, expectedDir);
}
1 change: 1 addition & 0 deletions test/cli/samples/config-async-function/rollup.config.js
Expand Up @@ -2,5 +2,6 @@ export default async () => ({
input: 'main.js',
output: {
format: 'cjs',
exports: 'auto'
},
});
2 changes: 1 addition & 1 deletion test/cli/samples/config-mjs-plugins/rollup.config.mjs
Expand Up @@ -7,6 +7,6 @@ import plugin from './plugin.mjs';

export default {
input: 'main.js',
output: { format: 'cjs' },
output: { format: 'cjs', exports: 'auto' },
plugins: [shebang(), replace({ ANSWER: 42 }), plugin(), nestedPlugin()]
};
Expand Up @@ -25,6 +25,7 @@ export default {
format: 'cjs',
dir: '_actual',
entryFileNames: 'cjs-[name].js',
exports: 'auto'
},
],
};
6 changes: 4 additions & 2 deletions test/cli/samples/context/_expected.js
@@ -1,6 +1,8 @@
'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

console.log(window);
var main = 42;
const foo = 42;

module.exports = main;
exports.foo = foo;
2 changes: 1 addition & 1 deletion test/cli/samples/context/main.js
@@ -1,2 +1,2 @@
console.log(this);
export default 42;
export const foo = 42;
6 changes: 4 additions & 2 deletions test/cli/samples/multiple-configs/rollup.config.js
Expand Up @@ -2,7 +2,8 @@ export default [{
input: 'main.js',
output: {
file: '_actual/bundle1.js',
format: 'cjs'
format: 'cjs',
exports: 'auto'
}
}, {
input: 'main.js',
Expand All @@ -13,6 +14,7 @@ export default [{
}],
output: {
file: '_actual/bundle2.js',
format: 'cjs'
format: 'cjs',
exports: 'auto'
}
}];
Expand Up @@ -5,11 +5,13 @@ export default {
output: [
{
format: 'cjs',
file: '_actual/main.js'
file: '_actual/main.js',
exports: 'auto'
},
{
format: 'cjs',
file: '_actual/minified.js',
exports: 'auto',
plugins: [terser()]
}
]
Expand Down
Expand Up @@ -4,6 +4,7 @@ export default {
{
format: 'cjs',
file: '_actual/cjs.js',
exports: 'auto',
sourcemap: true
},
{
Expand Down
3 changes: 2 additions & 1 deletion test/cli/samples/multiple-targets/rollup.config.js
Expand Up @@ -3,7 +3,8 @@ export default {
output: [
{
format: 'cjs',
file: '_actual/cjs.js'
file: '_actual/cjs.js',
exports: 'auto'
},
{
format: 'es',
Expand Down
2 changes: 1 addition & 1 deletion test/cli/samples/warn-import-export/_config.js
Expand Up @@ -7,7 +7,7 @@ module.exports = {
assertIncludes(
stderr,
'(!) Mixing named and default exports\n' +
'https://rollupjs.org/guide/en/#output-exports\n' +
'https://rollupjs.org/guide/en/#outputexports\n' +
'The following entry modules are using named and default exports together:\n' +
'main.js\n' +
'\n' +
Expand Down
2 changes: 1 addition & 1 deletion test/cli/samples/warn-mixed-exports-multiple/_config.js
Expand Up @@ -7,7 +7,7 @@ module.exports = {
assertIncludes(
stderr,
'(!) Mixing named and default exports\n' +
'https://rollupjs.org/guide/en/#output-exports\n' +
'https://rollupjs.org/guide/en/#outputexports\n' +
'The following entry modules are using named and default exports together:\n' +
'main.js\n' +
'lib1.js\n' +
Expand Down