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

Use entryFileNames when generating filenames for virtual modules #4270

Merged
merged 4 commits into from Nov 22, 2021

Conversation

BPScott
Copy link
Contributor

@BPScott BPScott commented Nov 14, 2021

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 #4269
Resolves rollup/plugins#1031
Resolves rollup/plugins#1037

Description

When output.preserveModules:true is set and there are virtual modules, rollup creates files on disk for each of those virtual modules.

Prior to this PR the filename was the sanitised virtual module id but no further transformation was done. This meant that if the virtual module id had no extension, then the filename on disk lacked an extension, and if did specify an extension then the filename on disk would always be that extension, even if output. entryFileNames tried to use a different filename.

This causes issues in cases where an environment can not load files that lack an extension (such as web browsers when the server can't set the correct mime-type - rollup/plugins#1037) or when the extension needs to be changed (such as when you want to create .mjs files for node esm support - rollup/plugins#1031)

With this PR, the filenames of virtual modules are processed with output.entryFileNames in the same way as non-virtual modules - which by default will add a .js extension to generated files and shall allow for customization of the extension of the filename.

Notes

This PR has been split into three commits to try and help ease reviewing

  • e67174e contains the fix itself within Chunk.ts
  • 8c5030d contains updates to all existing tests to reflect that files for virtual modules now get an extension by default:
    • The virtual module \0virtualModule previously generated _virtual/_virtualModule and now generates _virtual/_virtualModule.js
    • The virtual module \0other.js?commonjs-exports.js previously generated _virtual/_other.js_commonjs-exports and now generates _virtual/other.js_commonjs-exports.js
    • SystemJS references to modules include the .js extension, this seems to be consistent with how files produced from non-virtual modules are referenced
    • AMD references to modules omit the .js extension, but include other extensions, this seems to be consistent with how files produced from non-virtual modules are referenced
  • 8f3747b contains a new test that demonstrates the various entryFilenames keys shall be replaced in virtual filenames too

@BPScott BPScott changed the title Virtual module entryfilenames Use entryFileNames when generating filenames for virtual modules Nov 14, 2021
@codecov
Copy link

codecov bot commented Nov 14, 2021

Codecov Report

Merging #4270 (9d36805) into master (8d98341) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4270   +/-   ##
=======================================
  Coverage   98.39%   98.40%           
=======================================
  Files         204      204           
  Lines        7312     7319    +7     
  Branches     2084     2084           
=======================================
+ Hits         7195     7202    +7     
  Misses         58       58           
  Partials       59       59           
Impacted Files Coverage Δ
src/Chunk.ts 100.00% <100.00%> (ø)

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 8d98341...9d36805. Read the comment docs.

@BPScott BPScott force-pushed the virtual-module-entryfilenames branch from 61e0d7d to a49072f Compare November 14, 2021 14:11
- virtual files now have a .js extension by default
- systemjs module references include this file extension
- amd module references omit this file extension. this seems to be
  consistent with amd output of other chunks
Use entryFileNames to change the extension of the filename to be mjs,
and see that the file created from the virtual file chunk has a .mjs
extension

Note that the amd definition contains the .mjs extension, as amd ids only
strip the .js file extension, which is slightly odd but probably ok as
this amd is not the targetted usecase for this behaviour
@BPScott BPScott force-pushed the virtual-module-entryfilenames branch from a49072f to 8f3747b Compare November 14, 2021 14:15
@BPScott
Copy link
Contributor Author

BPScott commented Nov 14, 2021

Rebased atop latest master

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

Thanks, looks great!

@lukastaegert lukastaegert merged commit 9fc39d1 into rollup:master Nov 22, 2021
@BPScott
Copy link
Contributor Author

BPScott commented Nov 22, 2021

Thank you @lukastaegert <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants