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

UI: Add resolve.dedupe settings for vite #7251

Merged
merged 1 commit into from
Oct 19, 2022
Merged

UI: Add resolve.dedupe settings for vite #7251

merged 1 commit into from
Oct 19, 2022

Conversation

pleek91
Copy link
Contributor

@pleek91 pleek91 commented Oct 19, 2022

Description

We've been having quite a few issues when linking orion-design locally for development. Adding vue to vite's dedupe config seems to resolve those issues.

@pleek91 pleek91 added the enhancement An improvement of an existing feature label Oct 19, 2022
@netlify
Copy link

netlify bot commented Oct 19, 2022

Deploy Preview for prefect-orion ready!

Name Link
🔨 Latest commit e678d2e
🔍 Latest deploy log https://app.netlify.com/sites/prefect-orion/deploys/635051403d24ff000814b087
😎 Deploy Preview https://deploy-preview-7251--prefect-orion.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@nicolascribbles nicolascribbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱 nice - 1 liner

@znicholasbrown
Copy link
Contributor

Vue is listed as a peer dependency for orion-design so this solution is a little confusing... they should share the same installation, no?

@pleek91
Copy link
Contributor Author

pleek91 commented Oct 19, 2022

@znicholasbrown I don't totally understand but dedupe is a package feature. Npm even has its own dedupe flag. @stackoverfloweth tested trying to dedupe using npm and it didn't help. I found this in a vite github issue where someone was having the same issue we're experience and this was the suggested solution.

There are lots of issues with peer dependencies in general. But its my understanding that marking something as a peer dependency only enforces the consuming package meets that requirement. It doesn't change at all how the actual code is bundled or packaged. Similar to how rollup's external settings work. From my testing, setting vue as external (as described here) fixes this same issue when consuming published packages. But the issue can be reintroduced when using packages locally because of how npm treats symlinks and how vite is producing the bundles. The vite docs for resolve.dedupe mentions "linked packages in monorepos" as a reason you might want to use the dedupe setting.

I'm definitely not 100% confident in this fix. But from my testing it fixes the development issue we've been experience and doesn't seem to have any negative side effects with the build itself.

Copy link
Contributor

@znicholasbrown znicholasbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it @pleek91 - thank you for the explanation, that's helpful!

@pleek91 pleek91 merged commit 5fef509 into main Oct 19, 2022
@pleek91 pleek91 deleted the vite-dedupe branch October 19, 2022 20:08
@zanieb
Copy link
Contributor

zanieb commented Oct 19, 2022

@pleek91 if this is a user-facing enhancement, can you update the title to reflect the change in UX? If it's not user facing, can you change the label to maintenance so it is not included in the changelog?

@pleek91 pleek91 added development Tech debt, refactors, CI, tests, and other related work. and removed enhancement An improvement of an existing feature labels Oct 19, 2022
@pleek91
Copy link
Contributor Author

pleek91 commented Oct 19, 2022

@madkinsz updated the label. Thanks for staying top of these! I'll get it down soon hopefully 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Tech debt, refactors, CI, tests, and other related work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants