Use ES2015 features in generated code snippets

[lukastaegert]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:
Resolves #3784
Resolves #3092
Resolves #2591

Description

This will finally support ES6 features like arrow functions in generated Rollup code. For now, this is completely opt-in, see below. Furthermore, this fixes some long-standing bugs and adds a new internal concepts of "code-snippets", small functions to generate parts of the code respecting the new options.

As it turns out, blindly switching to ES6 features is not always good idea, which is why most specifically, the output.generatedCode.arrowFunctions option will leave larger IIFE functions in place to avoid double parsing in Chrome (see the link in the documentation below for the reasoning). This is also linked to a hidden feature in this PR

Improved AMD and SystemJS wrapper performance

Following https://v8.dev/blog/preparser#pife, immediately (directly or indirectly) invoked functions in AMD and SystemJS wrappers now receive the same treatment as their counterparts in UMD wrappers in that they are wrapped in parentheses. They are also never written as arrow functions as I could confirm that the same optimization does not apply to those.

Improved ES3 compatibility

While ES3 compatibility is generally not a goal of Rollup, we were already spending quite a bit of effort in quoting reserved words that are used as properties. This PR aims to complete this effort by quoting them in many more (all?) places that are controlled by Rollup. Props quoting can now be turned off via the output.generatedCode. reservedNamesAsProps option.

For the next major version, I would consider defaulting this option to true.

Improved ES5 compatibility + live-bindings for merged namespaces

Object.assign is no longer used to merge namespaces. Instead, a custom helper function is injected, which not only improves compatibility but also retains live-bindings when internal and external or synthetic namespaces are reexported from the same file.

Preserve "this"-context for AMD/CommonJS dynamic imports

Previously, dynamic imports containing a "this" in their argument would not preserve the context. This is fixed now in two ways:

• If arrow functions are allowed, we use an arrow function in the corresponding place (this was one of the drivers to add arrow functions)
• Otherwise we use an IIFE to retain context for fully dynamic imports (no checking is done if a "this" is used, though):

  (function (t) { return Promise.resolve().then(function () { return /*#__PURE__*/_interopNamespace(require(t)); }); })(this);

Fix shimMissingExports for internally used exports

As it turns out, shimMissingExports would fail to add the definition for the shim variable even though it is referenced for internally used imports.

New feature (copied from documentation): output.generatedCode

To get a feeling for what has changed, take a look at the expected test output here: https://github.com/rollup/rollup/tree/output-generated-code/test/form/samples/generated-code

Type: "es5" | "es2015" | { arrowFunctions?: boolean, constBindings?: boolean, objectShorthand?: boolean, preset?: "es5" | "es2015", reservedNamesAsProps?: boolean }
CLI: --generatedCode <preset>
Default: "es5"

Which language features Rollup can safely use in generated code. This will not transpile any user code but only change the code Rollup uses in wrappers and helpers. You may choose one of several presets:

• "es5": Do not use ES2015+ features like arrow functions, but do not quote reserved names used as props.
• "es2015": Use any JavaScript features up to ES2015.

output.generatedCode.arrowFunctions
Type: boolean
CLI: --generatedCode.arrowFunctions/--no-generatedCode.arrowFunctions
Default: false

Whether to use arrow functions for auto-generated code snippets. Note that in certain places like module wrappers, Rollup will keep using regular functions wrapped in parentheses as in some JavaScript engines, these will provide noticeably better performance.

output.generatedCode.constBindings
Type: boolean
CLI: --generatedCode.constBindings/--no-generatedCode.constBindings
Default: false

This will use const instead of var in certain places and helper functions. Depending on the engine, this can provide marginally better performance in optimized machine code. It will also allow Rollup to generate more efficient helpers due to block scoping.

// input
export * from 'external';

// cjs output with constBindings: false
var external = require('external');

Object.keys(external).forEach(function (k) {
  if (k !== 'default' && !exports.hasOwnProperty(k))
    Object.defineProperty(exports, k, {
      enumerable: true,
      get: function () {
        return external[k];
      }
    });
});

// cjs output with constBindings: true
const external = require('external');

for (const k in external) {
  if (k !== 'default' && !exports.hasOwnProperty(k))
    Object.defineProperty(exports, k, {
      enumerable: true,
      get: function () {
        return external[k];
      }
    });
}

output.generatedCode.objectShorthand
Type: boolean
CLI: --generatedCode.objectShorthand/--no-generatedCode.objectShorthand
Default: false

Allows the use of shorthand notation in objects when the property name matches the value.

// input
const foo = 1;
export { foo, foo as bar };

// system output with objectShorthand: false
System.register('bundle', [], function (exports) {
  'use strict';
  return {
    execute: function () {
      const foo = 1;
      exports({ foo: foo, bar: foo });
    }
  };
});

// system output with objectShorthand: true
System.register('bundle', [], function (exports) {
  'use strict';
  return {
    execute: function () {
      const foo = 1;
      exports({ foo, bar: foo });
    }
  };
});

output.generatedCode.preset
Type: "es5" | "es2015"
CLI: --generatedCode <value>

Allows choosing one of the presets listed above while overriding some options.

export default {
  // ...
  output: {
    generatedCode: {
      preset: 'es2015',
      arrowFunctions: false
    }
    // ...
  }
};

output.generatedCode.reservedNamesAsProps
Type: boolean
CLI: --generatedCode.reservedNamesAsProps/--no-generatedCode.reservedNamesAsProps
Default: false

Determine whether reserved words like "default" can be used as prop names without using quotes.

// input
const foo = null;
export { foo as void };

// cjs output with reservedNamesAsProps: false
Object.defineProperty(exports, '__esModule', { value: true });

const foo = null;

exports['void'] = foo;

// cjs output with reservedNamesAsProps: true
Object.defineProperty(exports, '__esModule', { value: true });

const foo = null;

exports.void = foo;

(Soft-) deprecation of preferConst

output.generatedCode.constBIndings should be used instead.

Use common helper for more invalid option errors

An effect of this is that all invalid option errors now contain a link to the corresponding documentation.

[github-actions]

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#output-generated-code

or load it into the REPL:
https://rollupjs.org/repl/?pr=4215

[codecov]

Codecov Report

Merging #4215 (462e985) into master (5700728) will increase coverage by 0.01%.
The diff coverage is 99.27%.

@@            Coverage Diff             @@
##           master    #4215      +/-   ##
==========================================
+ Coverage   98.37%   98.39%   +0.01%     
==========================================
  Files         202      203       +1     
  Lines        7260     7341      +81     
  Branches     2119     2087      -32     
==========================================
+ Hits         7142     7223      +81     
  Misses         58       58              
  Partials       60       60              

Impacted Files	Coverage Δ
cli/cli.ts	71.42% <ø> (ø)
cli/run/getConfigPath.ts	89.47% <ø> (ø)
cli/run/index.ts	100.00% <ø> (ø)
cli/run/loadConfigFile.ts	96.22% <ø> (ø)
cli/run/watch-cli.ts	92.10% <ø> (ø)
src/ExternalModule.ts	100.00% <ø> (ø)
src/ModuleLoader.ts	100.00% <ø> (ø)
src/finalisers/index.ts	100.00% <ø> (ø)
src/rollup/rollup.ts	100.00% <ø> (ø)
src/utils/PluginDriver.ts	100.00% <ø> (ø)
... and 46 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5700728...462e985. Read the comment docs.

[guybedford]

For the SystemJS module format at least, it is recommended to retain a function declaration for the execute and declaration functions since SystemJS can control the this bindings (eg setting this to null for modules). While RollupJS handles some of those replacements it won't handle them for eg eval cases.

If there is a slowdown for some specific "large IIFE" cases, it could also be worth sticking to function declarations there.

I haven't been able to review all cases, as the diff is very hard to read on the web app, but also worth being very careful about this bindings in every function replacement scenario!

[lukastaegert]

While RollupJS handles some of those replacements it won't handle them for eg eval cases.

But of course, eval is strongly discouraged to use in a bundling setup and we warn about it. Still, we could always use functions for execute (and I do not think we use declaration).

With regard to other function replacement scenario, e.g. in dynamic imports, here arrow functions actually fix long-standing bugs with regard to wrong this binding, notably #3092 .

However, this PR finally adds a clunky workaround for the function case as well.

[lukastaegert]

To get a feeling for what changed, take a look at the new test files here, this should be far better than browsing the diff: https://github.com/rollup/rollup/tree/output-generated-code/test/form/samples/generated-code

[shellscape]

Not stepping in to discourage or anything of the sort; I ran into this twitter thread by the Preact author the other day and thought it was good food for thought while pursuing this initiative - https://twitter.com/_developit/status/1437429523893600256

[lukastaegert]

I already reverted back to using ES5 syntax in the places where I am sure there would be a performance penalty. At this point, all remaining changes will provide either a small size reduction, or in the case of const instead of var, a theoretical small performance improvement though I cannot seem to find the article support this any more (the reasoning being that for const, the engine can drop some logic needed to handle reassignments, though the gain would be close to non-noticeable).

[lukastaegert]

However, there are mailing important refactorings and quite a few bug fixes that are also included here. Most importantly, I managed to get rid of quite a few places where Rollup was NOT generating strict ES5 output.

[lukastaegert]

This is finally ready for review! Have a look at the updated description, there are quite a few important fixes included as well.

[guybedford]

Excellent work, and glad to hear about the performance improvements.

I'd even consider making these features aggressive defaults - most users do want these features. Ideally a singular option for the target ecma version could apply across the board, for either enabling or disabling?

[lukastaegert]

Glad to hear you approve! I added the preset style to make this as easy as possible, i.e. generatedCode: "es2015" or generatedCode: "es5" would pull the switches for you.

most users do want these features

I thought so originally, but there is still a large group of library bundlers who still think "es5" is what should go into node_modules. And many tools still assume that this is the case, forcing you to add exceptions to the config if you need anything in node_modules transpiled. But let's see what we make the default once Rollup 3 becomes more of a reality.