Skip to content

chore: Fix vite-plugin-externalize-deps except field#47

Merged
lachlancollins merged 1 commit intomainfrom
rename-external-include
Feb 27, 2024
Merged

chore: Fix vite-plugin-externalize-deps except field#47
lachlancollins merged 1 commit intomainfrom
rename-external-include

Conversation

@lachlancollins
Copy link
Copy Markdown
Member

@lachlancollins lachlancollins commented Feb 27, 2024

  • include marks something as excluded
  • except marks something as to be bundled (confusing I know)
  • Renamed externalInclude to bundledDeps which is clearer IMO

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Feb 27, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 2aac4dc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@lachlancollins lachlancollins merged commit fe0f59e into main Feb 27, 2024
@lachlancollins lachlancollins deleted the rename-external-include branch February 27, 2024 08:54
@jprosevear
Copy link
Copy Markdown
Contributor

  • include marks something as excluded

I believe it adds additional dependencies to be externalized that were not coming from peer deps, deps, etc. I had to externalize a virtual module (I have a vite plugin that handles the loading of this module) - my suspicion is router may eventually need something like this too in the vite plugi.

https://github.com/davidmyersdev/vite-plugin-externalize-deps/blob/43e2091180139321b80163b05170ee53e727a0a4/src/index.ts#L25C3-L25C10

I chose includeExternals so that the api could later be extended with exceptExternals. I agree the semantics are a bit confusing because it "includes" additional externals and "excepts" externals (like from peer deps) that might otherwise

  • except marks something as to be bundled (confusing I know)
  • Renamed externalInclude to bundledDeps which is clearer IMO

This does the opposite of what I need, i do need access to 'include'. See my work around in #42 (comment)

@lachlancollins let me know how we can proceed (or if this is a behavior you prefer not to support). I'll hang out in discord for a bit.

@lachlancollins
Copy link
Copy Markdown
Member Author

This does the opposite of what I need, i do need access to 'include'. See my work around in #42 (comment)

I'm so sorry, that was a big oops on my side 🤦 I'll fix this now!

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.

2 participants