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

fix: externalize explicitly configured linked packages #9346

Merged

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