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

Add support for entryFileNames pattern used in combination with preserveModules option #3088

Merged

Conversation

Andarist
Copy link
Member

@Andarist Andarist commented Sep 1, 2019

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes
  • no

Breaking Changes?

  • yes
  • no

List any relevant issue numbers:
fixes #2847

Description

The main use case for this is to support "renaming" of the TS files to JS while transpiling.

I've skipped support for:

  • [ext]- let's wait to see if anybody really wants it before introducing this
  • [hash] - didn't have personal interest in implementing it and it wasn't straightforward for me how to implement this quickly, because computeContentHashWithDependencies expects addons argument, those were not passed into generateIdPreserveModules and I have no idea (without digging into this further) what those are

@codecov
Copy link

codecov bot commented Sep 1, 2019

Codecov Report

Merging #3088 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3088      +/-   ##
==========================================
+ Coverage   89.21%   89.22%   +0.01%     
==========================================
  Files         165      165              
  Lines        5719     5725       +6     
  Branches     1737     1738       +1     
==========================================
+ Hits         5102     5108       +6     
  Misses        380      380              
  Partials      237      237
Impacted Files Coverage Δ
src/Chunk.ts 92.8% <100%> (+0.08%) ⬆️
src/utils/assignChunkIds.ts 100% <100%> (ø) ⬆️

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 b226036...b31cced. Read the comment docs.

@Andarist Andarist force-pushed the preserve-modules/entry-file-names-pattern branch from 439591a to 5d94815 Compare September 1, 2019 07:26
@lukastaegert
Copy link
Member

Thanks for the PR! Just to let you know, I am a little busy with work-related things this week so a review may take until Friday. Thanks for your patience!

@Andarist
Copy link
Member Author

Andarist commented Sep 3, 2019

No worries

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 good! As for your questions:
addons are code snippets inserted via banner/footer/intro/outro. As for hashes in general, I agree that more changes would be necessary. I added a note to the documentation that this is not yet supported. If there is demand, this can always be added later, but at the moment I did not like the added complexity too much.

@lukastaegert lukastaegert merged commit 8dceaf6 into rollup:master Sep 8, 2019
@Andarist Andarist deleted the preserve-modules/entry-file-names-pattern branch September 8, 2019 10:57
@Andarist
Copy link
Member Author

Andarist commented Sep 8, 2019

q: how sanitizedId (based on first item in this.orderedModules) differs from this.getChunkName() (if not given it's based on last item in this.orderedModules)?

@lukastaegert
Copy link
Member

The key difference is that when the name is not explicitly specified (e.g. in the preserveModules case, as opposed to the case where you use an input object), this.getChunkName() will only be based on the "basename" via getAliasName, i.e. the last path fragment, while the sanitizedId is usually an absolute path.

@lukastaegert
Copy link
Member

With that in mind, one could argue that maybe people expect [name] to contain the path relative to the common base dir, but I think your interpretation is more useful.

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

Successfully merging this pull request may close these issues.

output.entryFileNames is ignored with preserveModules: true
2 participants