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

Improve absolute path handling #4021

Merged
merged 4 commits into from Mar 29, 2021
Merged

Conversation

lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Mar 27, 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 #3940
Resolves #3693
Resolves #3024

Description

This will very much extend the handling of external imports to allow having absolute external imports in the output. There are several parts to this:

  • A new option makeAbsoluteExternalsRelative that controls how absolute resolved ids are handled. By default it is true, which is the current behaviour. Switching it to false will keep them as absolute in the output while the third value "ifRelativeSource" will only convert them to a relative import if the original import has been relative (even though the resolved import might not have anything to do with the original import).

    "IfRelativeSource" will become the new default in the next major version

  • An extension of the resolveId plugin hook: When plugins return an object for their resolveId hook, they can now not only make the id "external" by returning external: true, they can also override the user's choice for the makeAbsoluteExternalsRelative option. By returning external: "absolute", they can force an absolute id to remain absolute in the output while by returning external: "relative", they can force it to become relative. For non-absolute ids, these choices will be treated as true.

  • A new value for the external property when using the this.resolve plugin context function. When an absolute id should opt out of becoming relative, the external property will have the value "absolute" instead of true. While this could be considered a breaking change, this will only happen if either the user or a plugin makes use of the new option. For that reason, this will still be a minor release.

See also the included doc updates.

@codecov
Copy link

codecov bot commented Mar 27, 2021

Codecov Report

Merging #4021 (8560404) into master (443dc97) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4021   +/-   ##
=======================================
  Coverage   97.47%   97.47%           
=======================================
  Files         192      192           
  Lines        6775     6785   +10     
  Branches     1985     1994    +9     
=======================================
+ Hits         6604     6614   +10     
  Misses         84       84           
  Partials       87       87           
Impacted Files Coverage Δ
src/Chunk.ts 100.00% <ø> (ø)
src/utils/options/mergeOptions.ts 100.00% <ø> (ø)
src/ExternalModule.ts 100.00% <100.00%> (ø)
src/ModuleLoader.ts 100.00% <100.00%> (ø)
src/utils/options/normalizeInputOptions.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 443dc97...8560404. Read the comment docs.

@lukastaegert lukastaegert force-pushed the improve-absolute-path-handling branch 2 times, most recently from 1a0dbe7 to 72ff786 Compare March 29, 2021 12:57
@github-actions
Copy link

github-actions bot commented Mar 29, 2021

Thank you for your contribution! ❤️

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

npm install rollup/rollup#improve-absolute-path-handling

or load it into the REPL:
https://rollupjs.org/repl/?circleci=14888

@rollup rollup deleted a comment from rollup-bot Mar 29, 2021
@lukastaegert lukastaegert force-pushed the improve-absolute-path-handling branch from 72ff786 to e7f6901 Compare March 29, 2021 13:36
@lukastaegert lukastaegert merged commit b7bb5e2 into master Mar 29, 2021
@lukastaegert lukastaegert deleted the improve-absolute-path-handling branch March 29, 2021 14:01
Copy link

@Oleksandr-prog Oleksandr-prog left a comment

Choose a reason for hiding this comment

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

?

@lukastaegert
Copy link
Member Author

While the question is rather vague, the answer to the three "fixed" issues is to add makeAbsoluteExternalsRelative: "ifRelativeSource". This will also become the default eventually. The complexity arises from the need to make it non-breaking as some libraries depend on the old behaviour.

marvinhagemeister added a commit to preactjs/wmr that referenced this pull request Aug 29, 2021
Looks like they made a breaking change in a minor release: rollup/rollup#4021
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