[v3.0] Better esm config file support #4123

lohfu · 2021-06-04T15:21:42Z

This PR contains:

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:

#4055

Description

Use a more precise version check to determine ESM support
Load .js config files untranspiled with import() if package.json has "type": "module"

Only checking if major version is above 13 leads to both false negatives and false positives. ESM is supported in ^12.17, ^13.2 and >=14. Thus, version 12.17 has support and 13.0 does not.

Rollup goes against specification when not loading .js config files as ESM despite "type": "module" being set in package.json. This actually causes imports of .cjs files to fail with:

Error: 'default' is not exported by ../../babel.config.cjs, imported by ../../rollup.config.js

Changing the extension of the Rollup file to .mjs resolves the issue.

Although this PR is a breaking change, it makes Rollup handle ESM config files the same way Node handles them natively. It will only make builds with "type": "module" and a .js Rollup config file with invalid ESM syntax fail. Moreover, projects with "type": "module" will already have spent time converting .js files (including config) to ESM or changing their extension to .cjs. If the Rollup config is not valid ESM, it's most likely because they forgot to check it or made a mistake.

github-actions · 2021-06-04T15:23:04Z

Thank you for your contribution! ❤️

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

npm install lohfu/rollup#better-esm-config-file-support

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

lukastaegert

I am not completely sure about this one, even for a major release. Maybe we actually make this much simpler and say we always use dynamic import(). This would work for CommonJS just as well as ESM, and then we just always add the interop check, i.e. roughly

const configFileExport = supportsNativeESM()
  ? getDefaultFromCjs(await import(pathToFileURL(fileName).href)).default)
  : extension === '.cjs'
  ? getDefaultFromCjs(require(fileName))
  : await getDefaultFromTranspiledConfigFile(fileName, commandOptions.silent)

That way, we do not rely on an additional package but directly use Node's algorithm to detect ESM/CommonJS. Thoughts?

lukastaegert · 2021-06-11T06:11:18Z

cli/run/loadConfigFile.ts

+	const major = parseInt(version[1], 10);
+	const minor = parseInt(version[2], 10);
+
+	return major >= 14 || (major === 13 && minor >= 2) || (major === 12 && minor >= 17);


That is definitely a reasonable improvement that I would merge even without a major release.

codecov · 2022-07-06T07:42:04Z

Codecov Report

Merging #4123 (9cbfb21) into release-3.0.0 (48ce34d) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@                Coverage Diff                @@
##           release-3.0.0    #4123      +/-   ##
=================================================
- Coverage          98.87%   98.87%   -0.01%     
=================================================
  Files                211      211              
  Lines               7391     7388       -3     
  Branches            2105     2103       -2     
=================================================
- Hits                7308     7305       -3     
  Misses                27       27              
  Partials              56       56

Impacted Files	Coverage Δ
cli/run/loadConfigFile.ts	`94.00% <ø> (-0.34%)`	⬇️

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 48ce34d...9cbfb21. Read the comment docs.

lukastaegert · 2022-07-06T08:08:59Z

As we are now finally starting work on a new major version, I revisited this PR, rebased it onto the release branch and applied some simplifications:

The version check will no longer be necessary as Node 14 will be the new baseline
I reduced the cases to "transpile or use import()" where the logic is
- We transpile (to CommonJS) if config plugins are used (because then we MUST transpile) or if the extension is .js and the type is not explicitly "module". The reason we transpile the latter is because there is a huge number of "legacy" code bases that use an ES module for Rollup's config file without setting "type": "module", and I would want to avoid the churn. We could consider warning in that case, though, but I am undecided. Thoughts?
- Otherwise we use import (even for the .cjs case for simplicity), checking is there is an __esModule flag on the default export (I feel that check should not cause too many issues for ESM files)

lukastaegert · 2022-07-06T08:10:21Z

Also, the relevant documentation will need to be reworked to better reflect the new logic.

…eprecated features (#4552) * Remove all active deprecations * Make all inactive deprecations active * Try to make test more stable * Update CLI help screen

* Initial new hashing idea * Simplify external import path generation 197 broken tests left * Use correct file names in chunk info 197 broken tests left * Implement first draft for hashing algorithm 189 broken tests left * Remove active deprecations this.emitAsset this.emitChunk this.getAssetFileName this.getChunkFileName import.meta.ROLLUP_ASSET_URL_ import.meta.ROLLUP_CHUNK_URL_ * Reduce render parameters * Always scan all chunks for hashes * Fix asset emission and remaining tests * Reintroduce augmentChunkHash and get OutputChunk by converting RenderedChunk * Provide chunk graph in renderChunk * Handle hash collisions * Remove deprecated hacky asset emission * Allow to configure hash sizes per file * Update documentation * Extend tests * Minor improvements * Improve documentation about hashing * Replace hash in sourcemap file * Provide ChunkInfo in banner/footer/intro/outro * Extract hashing logic * Clean up hashing logic * Add ExternalChunk wrapper * Store inputBase on Chunk * Store snippets on Chunk * Align chunk interfaces * Reduce this. property access * Move dynamicImportFunction warning to options normalization * Restructure rendering logic * Do not run on Node 10 * Update documentation * Try to fix Windows tests * Improve coverage * Remove graph background colors

* fix: run output plugins last * Add test Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>

* Convert scripts to ESM, update dependencies * Fix lint issue

lukastaegert · 2022-07-10T12:46:20Z

I updated the documentation now, mostly removing some sections and recommending to use rollup.config.mjs or add type: "module" in your package.json file. From my side this is good to go.

lukastaegert · 2022-07-30T13:00:48Z

This issue has been resolved via #4574 as part of rollup@3.0.0-3. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-3 or npm install rollup@beta. It will likely become part of a regular release later.

lukastaegert · 2022-08-15T04:46:03Z

This issue has been resolved via #4574 as part of rollup@3.0.0-4. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-4 or npm install rollup@beta. It will likely become part of a regular release later.

rollup-bot · 2022-08-31T05:28:58Z

This issue has been resolved via #4574 as part of rollup@3.0.0-5. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-5 or npm install rollup@beta. It will likely become part of a regular release later.

rollup-bot · 2022-09-06T05:31:43Z

This issue has been resolved via #4574 as part of rollup@3.0.0-6. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-6 or npm install rollup@beta. It will likely become part of a regular release later.

rollup-bot · 2022-09-23T04:48:04Z

This issue has been resolved via #4574 as part of rollup@3.0.0-7. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-7 or npm install rollup@beta. It will likely become part of a regular release later.

rollup-bot · 2022-10-11T04:34:08Z

This issue has been resolved via #4574 as part of rollup@3.0.0-8. Note that this is a pre-release, so to test it, you need to install Rollup via npm install rollup@3.0.0-8 or npm install rollup@beta. It will likely become part of a regular release later.

rollup-bot · 2022-10-11T13:43:27Z

This issue has been resolved via #4574 as part of rollup@3.0.0. You can test it via npm install rollup.

lukastaegert reviewed Jun 11, 2021

View reviewed changes

lukastaegert added this to In progress in Release 3.0.0 via automation Jun 22, 2021

lukastaegert mentioned this pull request Jul 2, 2021

Drop support for Node.js < 12 #4164

Closed

mreinstein mentioned this pull request Aug 9, 2021

Can rollup 2.x plugins be declared in pure esm form? #4203

Closed

lukastaegert changed the base branch from master to release-3.0.0 July 6, 2022 04:58

lukastaegert and others added 6 commits July 8, 2022 13:27

Deprecate Node 12 (#4548)

dc6d429

[v3.0] Remove actively deprecated features, show warnings for other d…

d080703

…eprecated features (#4552) * Remove all active deprecations * Make all inactive deprecations active * Try to make test more stable * Update CLI help screen

3.0.0-0

c0007f7

[v3.0] Run output plugins last (#3846)

9a5d32a

* fix: run output plugins last * Add test Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>

[v3.0] Convert build scripts to ESM, update dependencies (#4558)

5dcadfb

* Convert scripts to ESM, update dependencies * Fix lint issue

lukastaegert force-pushed the release-3.0.0 branch from 93d57da to 5dcadfb Compare July 8, 2022 11:31

lukastaegert and others added 6 commits July 8, 2022 13:31

3.0.0-1

48ce34d

More precise native ESM support check

fe952ec

Use import to load .js config file if package type is module

ab50d24

Update and add tests handling type module

fae7172

Remove Node version check and simplify logic

fa7a883

Update documentation

82928ac

lukastaegert approved these changes Jul 10, 2022

View reviewed changes

Document how to replace __dirname and import JSON

9cbfb21

lukastaegert changed the title ~~Better esm config file support~~ [v3.0] Better esm config file support Jul 11, 2022

lukastaegert moved this from In progress to Ready for merge in Release 3.0.0 Jul 11, 2022

lukastaegert reopened this Jul 12, 2022

Release 3.0.0 automation moved this from Ready for merge to In progress Jul 12, 2022

lukastaegert moved this from In progress to Ready for merge in Release 3.0.0 Jul 12, 2022

lukastaegert force-pushed the release-3.0.0 branch from 48ce34d to 11ee71f Compare July 15, 2022 10:58

lukastaegert mentioned this pull request Jul 15, 2022

[v3.0] Better esm config file support #4574

Merged

9 tasks

lukastaegert removed this from Ready for merge in Release 3.0.0 Jul 15, 2022

lukastaegert force-pushed the release-3.0.0 branch 2 times, most recently from 13b0ef8 to 46910f4 Compare July 20, 2022 07:25

lukastaegert force-pushed the release-3.0.0 branch from 46910f4 to d196046 Compare July 29, 2022 11:29

lukastaegert closed this Jul 30, 2022

lukastaegert mentioned this pull request Jul 30, 2022

[v3.0 Release branch] #4549

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v3.0] Better esm config file support #4123

[v3.0] Better esm config file support #4123

lohfu commented Jun 4, 2021 •

edited

github-actions bot commented Jun 4, 2021 •

edited

lukastaegert left a comment

lukastaegert Jun 11, 2021

codecov bot commented Jul 6, 2022 •

edited

lukastaegert commented Jul 6, 2022

lukastaegert commented Jul 6, 2022

lukastaegert commented Jul 10, 2022

lukastaegert commented Jul 30, 2022

lukastaegert commented Aug 15, 2022

rollup-bot commented Aug 31, 2022

rollup-bot commented Sep 6, 2022

rollup-bot commented Sep 23, 2022

rollup-bot commented Oct 11, 2022

rollup-bot commented Oct 11, 2022

[v3.0] Better esm config file support #4123

[v3.0] Better esm config file support #4123

Conversation

lohfu commented Jun 4, 2021 • edited

Description

github-actions bot commented Jun 4, 2021 • edited

Thank you for your contribution! ❤️

lukastaegert left a comment

Choose a reason for hiding this comment

lukastaegert Jun 11, 2021

Choose a reason for hiding this comment

codecov bot commented Jul 6, 2022 • edited

Codecov Report

lukastaegert commented Jul 6, 2022

lukastaegert commented Jul 6, 2022

lukastaegert commented Jul 10, 2022

lukastaegert commented Jul 30, 2022

lukastaegert commented Aug 15, 2022

rollup-bot commented Aug 31, 2022

rollup-bot commented Sep 6, 2022

rollup-bot commented Sep 23, 2022

rollup-bot commented Oct 11, 2022

rollup-bot commented Oct 11, 2022

lohfu commented Jun 4, 2021 •

edited

github-actions bot commented Jun 4, 2021 •

edited

codecov bot commented Jul 6, 2022 •

edited