szymmis/vite-express

Loading config using Vite is causing issues

Closed this issue · 8 comments

Loading Vite config in production causes issues in environments that purge dev dependencies. I think it would be better to not load Vite in production at all. See following pattern from vite-plugin-ssr: https://github.com/brillout/vite-plugin-ssr/blob/1a2e701de69314b33d76f5e28963b20c4c92c384/boilerplates/boilerplate-react-ts/server/index.ts#L16-L28

CleanShot 2023-05-31 at 11 49 48@2x

Yeah, when NODE_ENV is set to production dev dependencies are not installed with npm/yarn. The problem is that in vite-express we are using Vite to resolve dist files directory, so I don't see how would it be possible to resign from importing Vite production mode unfortunately.

I know that this kind of solution might not be satisfying but easy fix for you right now is to move Vite's dependencies as production deps. Your other options is to import and use vite-express only in dev mode and in production mode serve static files manually.

Maybe I should read vite.config manually without using Vite API but I would need to check out how Vite is loading it to see if there are some corner cases that need to be considered. Let me know what do you think @patryk-smc.

Not using Vite in production mode seems like a good idea after all because with plain Vite your final step is to build static files using vite build command and then it is up to you how you do this, so Vite is not involved in production at all.

@szymmis Since it's just about pointing to the dist folder, I ended up hard-coding the path in my fork. For but the lib users, it seems to me that it would make sense to have them provide the path to the dist folder.

I want to abstract as much as I can from the users, because otherwise it is just better to use vite + express on your own to have full control on this. That's why I don't want to make them provide the path themselves, I want to have it integrated nicely with Vite.

I want to abstract as much as I can from the users, because otherwise it is just better to use vite + express on your own to have full control on this. That's why I don't want to make them provide the path themselves, I want to have it integrated nicely with Vite.

Totally agree! Maybe default to "/dist". If not detected - throw an error letting them know that if they have it in different location, they need to provide path as parameter

Do you have vite.config.js in your production environment or is it a file that you omit? From the screen you sent I guess that you have that file present even when serving from heroku.

I'm asking about this because I want to know if it is a good idea to read vite.config.js directly as a text file and extracting dist path that way, because that's really the only thing I need from the config in production mode right now. That way I'm dropping the dependency for Vite in production mode and we can even make some defaults as you said (/dist as it is the default in Vite), but we don't need user to specify it in other place than the config itself.

I guess for 90% of use cases the default /dist is enough so some kind of ViteExpress.config option (basically what you suggested) isn't a bad idea either, but I'd like to have as little additional config as possible, and take what I can from Vite's config file.

Okay I've figured out some solution and would be grateful if you could tell me what do you think about and if that solves your use-case. By default everything still works the same, but when you specify inline config using ViteExpress.config() you can make vite-express run in viteless in production mode so that you don't need Vite and all those plugin dependencies in deployment. See #64 for more information, and also ask here if you have any questions or suggestions. You can also check out changes to README in that PR and tell me if that makes sense. Thanks a lot for you suggestion!

I've just released v0.9.0 with changes presented in #64 so you can check it out and let me know if there are some problems or possible improvements. Thanks a lot for raising this issue @patryk-smc. Always great to hear suggestions!