Skip to content
This repository has been archived by the owner on Jan 4, 2023. It is now read-only.

feat: build dev server bundle using vite #201

Merged
merged 20 commits into from
Sep 30, 2021
Merged

feat: build dev server bundle using vite #201

merged 20 commits into from
Sep 30, 2021

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Sep 9, 2021

In the initial POC, we started using rollup to build server bundle in development mode (using vite.build) which is probably not the best idea since rollup is intended to be used for production builds of vite and by doing so, other than performance downsides introduces lots of unwanted behaviors from plugins in development.

Vite supports server side rendering but in order to use it, we need to use vite.ssrLoadModule that directly loads a module but this approach is neither compatible with nuxt2 that is using vue-bundle-renderer nor new nitro pipeline since both expect to receive a bundle code (string) for at least entry point to execute but ssrLoadModule directly gives a module instance.

This PR is trying to use the same viteServer (instead of rollup) and transformRequest (id, { ssr: true }) API to produce the same code from the module graph for building a compatible server bundle in development.

Current issues with PR

  • Vite server produces ESM code (with resolved ESM externals) which is incompatible with vue-bundle-renderer (of nuxt2) that uses a CJS wrapper to execute bundle
  • vueComponentNormalizer is empty (related to vite-plugin-vue2?)
  • TypeError: Cannot read property 'get' of undefined Error when using transformRequest (css-post plugins crashes when using transformRequest without first starting a server vitejs/vite#3798)
  • antfu: Dependencies are cached in Node, which causing Vue.use to be called multiple times when re-requesting the pages (can't redefine '$router', [vue-composition-api] already installed)
  • Watch server

Future improvements

  • Once we could support basic rendering, we can try lazy compilation of server entry dependencies in a way that is compatible with WP5 lazy compilation and Nuxt3/Nitro (using fetch). Also for Nuxt3 and Nuxt2Bridge implementations (when backporting) we can try to directly use native ESM since nitro worker is a module already.
  • Internal bundle graph can be cached if there is a flag in vite to invalidate updated requests.
  • Sourcemap support
  • Use a shared viteServer instance in development (this needs a way to set different aliases for client and server. maybe with a plugin replacing define config?)
  • Composition API on server seems to be broken

@antfu
Copy link
Member

antfu commented Sep 9, 2021

@pi0 I have made some updates, and finally got it running (on the first render)! I have tried to make each commit small and self-explainable, you can read them to catch up.

But I also found a more critical problem which I have listed in the Current issues section:

antfu: Dependencies are cached in Node, which causing Vue.use to be called multiple times when re-requesting the pages (can't redefine '$router', [vue-composition-api] already installed)

AFAIK, there is no clear way to clean the cache for ESM (or do we with jiti?). Do you can any idea on this?


Re: ESM bundle

The ESM externals really tricky, and I didn't find a quick solution for that. For now, could we just use jiti for it, or are there any problems with it? Perf?

Re: Watch

server.watcher could do I think

Re: Cache

Vite has a built-in module graph with proper cache and invalidation implemented already. So I guess we don't need to do anything about that.

For invalidation:

const mod = server.moduleGraph.getModuleById(id)
mod && server.moduleGraph.invalidateModule(mod)

Re: Shared server:

Is that possible for us to have the same alias for both mode? If you really need that, I guess we could write an inline plugin for it (but that could possibly burst the cache I guess)

@pi0
Copy link
Member Author

pi0 commented Sep 10, 2021

Thanks for helping on this @antfu <3

Regarding the cached version of node_modules, we by default in nuxt2 use runInNewContext to isolate each render, helping to avoid memory leaks and dependency caches (I've temporary disabled it here)

We have 4 possible methods for clearing node_modules cache:

  • When using jiti, it will load all ESM dependencies into require.cache so that we can clear it with some package like clear module or nuxt/utils/clearRequireCache
  • For native module support (which is not a part of this PR goals) we can use unjs/mlly#loadModule I made this POC specifically to overcome this issue
  • For nitro, since we use a new Module Worker for each reload, there are no deps cached
  • We can avoid externalizing node_modules with transformRequest and let vite's cache do the trick

Is that possible for us to have the same alias for both mode? If you really need that, I guess we could write an inline plugin for it (but that could possibly burst the cache I guess)

Indeed that is tricky. Since for making nuxt plugins or ssr/client only logic at least, we use if (process.server) check that basically forces bundler tree-shake that part from client by statically replacing process.server to if(false) in code. We can talk about it later for now two instances seems good enough :)

src/server.ts Outdated Show resolved Hide resolved
@antfu
Copy link
Member

antfu commented Sep 13, 2021

For the dependency caching, I guess it will be more straightforward to use jiti until we have native esm support. @pi0 can you share some pointers on how to clear the cache of jiti? Thanks

@pi0
Copy link
Member Author

pi0 commented Sep 13, 2021

@antfu Jiti uses shared CJS cache which nuxt should normally clear (if not, see #201 (comment)) but is probably adding huge perf impact of babel... :( I wish we can try mlly or bundling approaches to see the best.

@antfu
Copy link
Member

antfu commented Sep 14, 2021

I tried 27f6545 but the is still there. Any idea on that?

For the perf, maybe we could have a mode in jiti using https://github.com/alangpierce/sucrase? It's pure JS and claims to be even faster than swc/esbuild.

For mlly, I am not sure if I understand the usage here, doesn't it support ESM only?

@pi0
Copy link
Member Author

pi0 commented Sep 14, 2021

For the perf, maybe we could have a mode in jiti using https://github.com/alangpierce/sucrase? It's pure JS and claims to be even faster than swc/esbuild.

Seems a good idea to try for jiti (~> unjs/jiti#40 )👍 Still basically we are adding an unnecessary process step just for sandbox incompatibility with ESM....

For mlly, I am not sure if I understand the usage here, doesn't it support ESM only?

We can use dynamic imports in supported Node.js also in a CJS context (in wrapper) With mlly, we can directly evaluate an ESM bundle (also from remote sources for lazy compilation in nitro). Since we use inline base64, cache is automatically bypassed.

@antfu
Copy link
Member

antfu commented Sep 16, 2021

The problem here is that mlly itself is ESM-only, so is there a way to wrap it in CJS? I tried only apply jiti to mlly and use loadModule for server.mjs but no luck.

Maybe we should solve the root case instead? For forking the vue-renderer or require nitro to be used with this package?

const __vite_ssr_exports__ = exports;
const __vite_ssr_exportAll__ = __createViteSSRExportAll__(__vite_ssr_exports__)
${res.code || '/* empty */'};
return module.exports;
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for @antfu: Vite transform is not handling CJS at all (Node.js and Rollup both have basic CJS-in-ESM proxy support). Had to do this because some node_modules use CJS style exports but we also have this issue for the browser side. Maybe escalating this to add basic support to vite or refactor as a vite transform plugin?

@pi0
Copy link
Member Author

pi0 commented Sep 20, 2021

The last change is using CJS bundle format and inlining all node_modules. This has slight performance downside that we each time include all vendors but resolves many esm-cjs related issues including cache.

@pi0

This comment has been minimized.

@pi0 pi0 marked this pull request as ready for review September 30, 2021 19:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants