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

Also pass resolved ids to external if they use the object form #3753

Merged
merged 1 commit into from Aug 29, 2020

Conversation

lukastaegert
Copy link
Member

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:
#3743

Description

As it turned out, when a plugin resolves an id using the object form, the resolved id is not again passed to the external option for a second check, which is fixed here. Thus, the examples in the documentation work again :)

@rollup-bot
Copy link
Collaborator

Thank you for your contribution! ❤️

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

npm install rollup/rollup#pass-resolved-objects-to-external

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

@codecov
Copy link

codecov bot commented Aug 29, 2020

Codecov Report

Merging #3753 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3753   +/-   ##
=======================================
  Coverage   97.02%   97.02%           
=======================================
  Files         185      185           
  Lines        6461     6461           
  Branches     1868     1868           
=======================================
  Hits         6269     6269           
  Misses        101      101           
  Partials       91       91           
Impacted Files Coverage Δ
src/ModuleLoader.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 7e2cbc5...7b41291. Read the comment docs.

@lukastaegert lukastaegert merged commit 21fcec3 into master Aug 29, 2020
@lukastaegert lukastaegert deleted the pass-resolved-objects-to-external branch August 29, 2020 06:33
benjamn added a commit to apollographql/apollo-client that referenced this pull request Jun 1, 2021
Although the upgrade from rollup@1.31.1 to rollup@2.x has been almost
entirely seamless (yay!), rollup@2.26.8 included a change
(rollup/rollup#3753) that made it possible for
the options.external function to receive fully resolved, absolute ID
strings, so our implementation of that function needed to be updated to
accommodate that possibility.
benjamn added a commit to apollographql/apollo-client that referenced this pull request Jun 1, 2021
Although the upgrade from rollup@1.31.1 to rollup@2.x has been almost
entirely seamless (yay!), rollup@2.26.8 included a change
(rollup/rollup#3753) that made it possible for
the options.external function to receive fully resolved, absolute ID
strings, so our implementation of that function needed to be updated to
accommodate that possibility.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants