- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Exclude dependencies from non-CDN builds #3459
Exclude dependencies from non-CDN builds #3459
Conversation
Marks packages as "external" for non-CDN builds.
I'm all about this |
@jameschensmith thanks for the PR! Makes sense to me. Can you explain why |
@joshhanley, no problem! Yes, it is related to the changes. Because the non-CDN builds no longer have the dependencies bundled with them, |
@jameschensmith ok, great! Thanks for that 🙂 |
Thanks! |
This change seems to cause issues in our project, after upgrading ERROR in ./node_modules/alpinejs/dist/module.esm.js 1562:0-88
Module not found: Error: Can't resolve '@vue/reactivity' in '/[path-to-project]/node_modules/alpinejs/dist' Even adding |
That's quite strange. I'd try deleting node modules and package-lock and reinstalling. Which bundler are you using? Maybe you're not using node module resolution |
Removing the node_modules had no effect. We're using Laravel Mix (Webpack) and the But if this change would require one to have |
It doesn't and wouldn't. Although, many would say using webpack and cjs was already broken. what is important is the module resolution, not the type of module. Basically that it should walk up the tree checking for But if you are not using type module, and it is still going to |
I think I've found the issue, thanks @ekwoka for pointing me in the right direction :D We have an alias defined, |
That would do it!!!! This is why it's most common to have your aliases be |
It seems that this PR caused bundling issues. I reverted it. |
@calebporzio can you elaborate on what were the issues that made you revert those changes, besides the aliases conflicts in @RobertBoes project? i'd like to bring this PR to life, since it's a very non-npm way of doing things to ship bundled dependencies in npm packages, since it's kinda like shipping CDN-ish build, but to NPM, it makes updating the underlying deps impossible without falling back to use and because of this unwanted bundling exists, some packages ( Update regarding the issues that caused the revert - it was #3602, and I'm kinda puzzled, because it was a simple change, yet you decided to reject it, and go back to keep using bad practices, just because they were working before |
Summary
Marks packages as "external" for non-CDN builds. Currently, if you install
v3.12.0
of alpinejs, you will pull down@vue/reactivity
and it's dependency intonode_modules
while also having it bundled within alpinejs. This PR excludes those dependencies only from the ESM / CJS bundles.See related discussion / recommendation here.