Skip to content

fix: externalize explicitly configured linked packages#9346

Merged
patak-dev merged 3 commits intomainfrom
fix/externalize-explicitily-configured-linked-packages
Jul 25, 2022
Merged

fix: externalize explicitly configured linked packages#9346
patak-dev merged 3 commits intomainfrom
fix/externalize-explicitily-configured-linked-packages

Conversation

@patak-dev
Copy link
Member

@patak-dev patak-dev commented Jul 24, 2022

Description

Follow up to:

After this PR, vite-plugin-ssr CI fails in vite-ecosystem-ci

When a package is explicitly configured as external, externalize it even if it is linked (it isn't in node_modules).

This PR also checks if a package is resolvable before externalizing it. I think this is more consistent between direct package imports and imports of package entries.

The PR also avoids returning undefined when calling tryNodeResolve with the externalize flag. Instead, it returns the resolved object, and in the ssr logic, we check for its external flag.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-dev patak-dev requested a review from bluwy July 24, 2022 21:30
@patak-dev patak-dev added the p4-important Violate documented behavior or significantly improves performance (priority) label Jul 24, 2022
@patak-dev
Copy link
Member Author

vite-plugin-ssr is passing with this PR:

And same for other CIs in the ecosystem, except for SvelteKit:

@bluwy maybe you could check what is happening there? Also, if you have other ideas for this PR, please go ahead, I tried to keep the current structure for this fix. I still think we will need a refactoring in 3.1

@patak-dev patak-dev merged commit c33e365 into main Jul 25, 2022
@patak-dev patak-dev deleted the fix/externalize-explicitily-configured-linked-packages branch July 25, 2022 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p4-important Violate documented behavior or significantly improves performance (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants