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

Vite: Fix lodash imports with optimizeDeps #21528

Closed
wants to merge 1 commit into from
Closed

Vite: Fix lodash imports with optimizeDeps #21528

wants to merge 1 commit into from

Conversation

thtliife
Copy link
Contributor

lodash imports using the file extension cause an error in vite unless included in the optimizeDeps.include array in the viteFinal config for storybook.

Closes: #21527

What I did

This commit replaces lodash imports to exclude the file extension.

eg.

before: import camelCase from 'lodash/camelCase.js';

after: import camelCase from 'lodash/camelCase';

How to test

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

lodash imports using the file extension cause an error in vite unless
included in the `optimizeDeps.include` array in the viteFinal config for
storybook.

This commit replaces lodash imports to exclude the file extension.

eg.

before: `import camelCase from 'lodash/camelCase.js';`

after: `import camelCase from 'lodash/camelCase';`

closes: #21527
@shilman shilman changed the title fix: lodash imports force vite optimizeDeps usage Vite: Fix lodash imports with optimizeDeps Mar 10, 2023
@shilman shilman requested a review from ndelangen March 10, 2023 07:02
@JReinhold
Copy link
Contributor

There was a PR specifically for adding .js extensions to lodash by @IanVS, because it fixed an issue. I'm not sure it is straight forward to just remove them again.

#20443

@ndelangen
Copy link
Member

@thtliife I think my PR here: #21535 supersedes this PR.
Removing the extensions is not desired, I think.

@thtliife thtliife closed this Mar 10, 2023
@thtliife
Copy link
Contributor Author

@thtliife I think my PR here: #21535 supersedes this PR.

Removing the extensions is not desired, I think.

Fair enough. I know that vite seems to treat paths with an extension as if they are project files, that's why I took the route of removing them.
I have closed the PR though and guess I'll stick to manually adding deps to the optimzedDeps array to keep my storybooks working 🙂

@IanVS
Copy link
Member

IanVS commented Mar 10, 2023

@thtliife thanks for the contribution. We'll get it fixed for sure, you shouldn't have to change your optimizedDeps array. Currently, rc.1 does not have this bug, so feel free to update to that version, unless you need yarn pnp support, then you'll need to update your array as you mention and wait for a fix to be released.

@thtliife
Copy link
Contributor Author

@IanVS Ahh nice, glad it's fixed already 🙂

I WISH I needed yarnpnp support...
My project has too many other deps that aren't pnp compatible...
Not many other repos are as responsive as you guys 🤪

@thtliife thtliife deleted the fix-ambiguous-indirect-export-default branch March 10, 2023 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants