-
-
Notifications
You must be signed in to change notification settings - Fork 488
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
refactor: add Vite compatibility when lazy loading #1084
Conversation
I'm fine with it although I would love if there wouldn't have to be a special case checking for the presence of nuxt-vite. Do you have some links that explain that |
I asked pooya to check it, I wanted to make sure about checking
It is the second reason why I keep this drafted and asked Pooya. |
My bad, your refactor is great :) I was addressing |
Ideally, Vite would also support the (sorry, I've removed my earlier comment as I've realized that you meant something else) |
@farnabaz @rchl At least at this stage, we should not depend on any vite-specific feature like checking it's existence from buildModules or
Actually it should do since we are providing resolve.alias option for Let me check PR locally :) |
Thanks to both of you for your work on that. |
src/templates/options.js
Outdated
<%= options.locales.map(l => `'${l.file}': () => import('../${relativeToBuild(options.langDir, l.file)}' /* webpackChunkName: "lang-${l.file}" */)`).join(',\n ') %> | ||
<%= options.locales | ||
.map(l => `'${l.file}': () => import('../${relativeToBuild(options.langDir, l.file)}' /* webpackChunkName: "lang-${l.file}" */)`) | ||
.filter((l, index, self) => self.indexOf(l) === index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be done in a simpler approach by mapping to file and use a Set 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to tested but looks good to me now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM too. I've tested it in one of my projects that uses lazy loading and of course tests pass also.
Haven't tested with Vite though so I can wait until someone does that. |
There is no pressure on vite. Since everything is super experimental know so i don't want to make chaos in community for vite support. These PRs generally improved runtime quality with esm support. |
Using the module with Nuxt Vite does not lazy load languages on client site because of limitations of dynamic import.
This PR will expand dynamic import into a switch case to remove variables inside import name and automatically detects build target and change the
srcDir
alias@pi0 What do you think about this?