search

solidjs / solid-start

SolidStart, the Solid app framework
https://start.solidjs.com
MIT License
5.06k stars 377 forks source link

Slim down package.json deps for new projects? #989

Closed devinrhode2 closed 1 year ago

devinrhode2 commented 1 year ago

Originally titled "Remove 6 un-necessary package.json dependencies for new projects"

Duplicates

Latest version

Current behavior 😯

FYI: Search "remove un-necessary deps vite esbuild" gave no results.

npm init solid@latest includes several deps that aren't needed for npm run dev/build/start

Expected behavior 🤔

New projects' package.json files only include necessary deps

Steps to reproduce 🕹

Steps:

  1. npm init solid@latest
  2. Remove these: esbuild, postcss, solid-js, @solidjs/router, @solidjs/meta, vite
  3. Run npm i
  4. Run npm run dev/build/start
  5. Observe everything works without these deps.

Context 🔦

New projects (with SSR and typescript) could have a package.json file like this:

     "start": "solid-start start"
   },
   "type": "module",
+  "optionalDependencies:comment": [
+    "esbuild uses `optionalDependencies` to install platform-specific binaries.",
+    "Using `npm install --omit=optional` will cause issues with esbuild."
+  ],
   "devDependencies": {
     "@types/node": "^18.16.19",
-    "esbuild": "^0.14.54",
-    "postcss": "^8.4.26",
     "solid-start-node": "^0.3.0",
-    "typescript": "^4.9.5",
-    "vite": "^4.4.6"
+    "typescript": "^4.9.5"
   },
+  "deps:comment": [
+    "These are needed for `npm run start` in production."
+  ],
   "dependencies": {
-    "@solidjs/meta": "^0.28.5",
-    "@solidjs/router": "^0.8.2",
-    "solid-js": "^1.7.9",
     "solid-start": "^0.3.1"
   },
   "engines": {

Your environment 🌎

what command?
MariuzM commented 1 year ago

Hm not sure if this is a bug or more like feature request

edivados commented 1 year ago

Tried a couple of things before to reduce docker image size. This will not work with pnpm because vite and solid-js will not be found.

The minimum I could get away with on example-bare with npm/pnpm is below. I have not tested this extensively so it may not work on some package managers or versions. Additionaly moving solid-start to devDependencies saves ~90MB of node_modules in prod.

There may be valid reasons to have explicit verisons of postcss and esbuild installed. After taking a second look, maybe there is no reason to do it by default . esbuild is a dependency of solid-start and postcss is a dependency of vite.

I don't understand the package resolution business so well and am confused right now why pnpm does not complain about meta and router like it does for vite and solid-js.

{
  "name": "bare",
  "scripts": {
    "dev": "solid-start dev",
    "build": "solid-start build",
    "start": "solid-start start"
  },
  "type": "module",
  "devDependencies": {
    "@types/node": "^18.16.19",
    "solid-start-node": "^0.3.0",
    "typescript": "^4.9.5",
    "vite": "^4.4.9",
    "solid-js": "^1.7.11",
    "solid-start": "^0.3.1"
  },
  "dependencies": {

  },
  "engines": {
    "node": ">=18"
  }
}
devinrhode2 commented 1 year ago

I'm using npm 9, bundled with Node 18.14,2

I guess we should add docs on production deployments.

I create a build outside docker, and then copy on just the dist folder. Pedantically, I have a prod-package.json which I copy in as package.json. It includes just solid-start and some "@suid/vite-plugin" used in my vite.config.ts, as dependencies. It includes a "start": "solid-start start", npm script along with "type": "module", and that's it.

inside docker, I also copy in my package-lock.json, and my prod-package.json uses "*" semver specs for solid-start a @suid/vite-plugin. Npm doesn't pickup the latest when installing these two packages via npm ci. It only uses the version set in package-lock.json.

Imo dependencies should at least be used for deps needed to run production build (solid-start start).

Additionally, I think deps needed to create a production build could also be included.

If you're creating a library, then I think you should only have devDeps. I suppose ultimately there could be an extra question/option when running create-solid, "Are you creating a library?", etc.

I guess we could have a question on what package manager people are using... But who wants to write that code and maintain it? I think a universal package.json is better, so I can easily switch package managers.

I'll add one note about yarn v1: although lots of legacy projects use it, and people can absolutely keep using it, I've found a few situations where the most reasonable thing is to ignore the warnings. I don't recall ever personally fixing any actual issues by addressing yarn v1's peer dep warnings, but maybe it's just a matter of constructing the right situation where an error will occur.

edivados commented 1 year ago

Imo dependencies should at least be used for deps needed to run production build (solid-start start).

So far I don't think solid-start is required in production and it does not make sense to me to install it just for the command. The start command calls the adapter's start function and what it does depends on the adapter's implementation. Start is more of a preview build command to me and not what I would call in prod.

When building with the node adapter I would call node dist/server.js which is basically what the node adapter does by importing server.js.

The only cases I can think of to have in deps are server only dependencies that cannot be bundled with the server and require to be externalized or don't want to bundle for whatever reason.

If you're creating a library, then I think you should only have devDeps. I suppose ultimately there could be an extra question/option when running create-solid, "Are you creating a library?", etc.

For a library you probably have different requirements and define solid-start as a peerDep etc. There is work going on for a solid-cli. Maybe something to look at there?

devinrhode2 commented 1 year ago

For a library you probably have different requirements and define solid-start as a peerDep etc. There is work going on for a solid-cli. Maybe something to look at there?

I'm not sure, I'm not building a library (npm package) with Solid/SolidStart, I'll have to let others drive any efforts there.

When building with the node adapter I would call node dist/server.js which is basically what the node adapter does by importing server.js.

I will certainly switch to doing this! This should alleviate the need for my prod-package.json :)

devinrhode2 commented 1 year ago

One more note/question: I noticed when solid-start start was running in my prod docker container, it didn't see any ui routes or api routes.

I COPY'd the src folder into the docker container, and viola, routes are being registered. It also seemed to be the case that we need esbuild to run the production server, because it's transpiling this src directory on the fly (hopefully with caching?).

I did explore moving my npm run build into docker, after having noticed some vite docs mentioning that vite will create some directories with pre-bundled deps. However, if I add a clean node_modules folder into git, and then create a build, nothing is added/changed in node_modules. I discovered my .solid directory is only related to the vite-plugin-inspect tool, which is driven by the defineConfig({ inspect: boolean }) flag. Setting inspect: false leads to no .solid directory being created (or the directory is empty). I concluded that I don't need to run npm run build inside docker, because it doesn't pre-bundle anything, all I need is the dist folder inside docker.

I'd imagine in the future, we will want to pre-compile the src folder, and avoid requiring esbuild to run a production server, working towards something like Next.js's standalone feature.

devinrhode2 commented 1 year ago

Overall, I think it'd be nice to document, and setup a contract, for production deployments.

There could even be a separate community-maintained Docker image and npm/shell scripts.

edivados commented 1 year ago

One more note/question: I noticed when solid-start start was running in my prod docker container, it didn't see any ui routes or api routes.

I COPY'd the src folder into the docker container, and viola, routes are being registered. It also seemed to be the case that we need esbuild to run the production server, because it's transpiling this src directory on the fly (hopefully with caching?).

Yeah, the compiled server does not print any routes. Printing of the routes is done by the solid-start cli on the start command by using the router. This requires vite etc. things not needed in production. This is why I refer to the start command as preview build.

https://github.com/solidjs/solid-start/blob/c11e275114a386befcf51013674d48437a9a93a9/packages/start/bin.cjs#L471-L481

However, if I add a clean node_modules folder into git, and then create a build, nothing is added/changed in node_modules

  1. Don't add node_modules to git
  2. The depdencies are in that case already there I guess

I concluded that I don't need to run npm run build inside docker, because it doesn't pre-bundle anything, all I need is the dist folder inside docker.

No, you don't want to build inside your docker image or at least not on the final build stage if using multi stage builds.

devinrhode2 commented 1 year ago
  1. Don't add node_modules to git

yes 🤣 - I was only doing this temporarily, to double check if solid-start build is adding anything to a node_modules/.cache or node_modules/.vite directory.

edivados commented 1 year ago

Oh got it. I also read npm install for some reason and focused on the package installation. 😄

devinrhode2 commented 1 year ago

Yeah, the compiled server does not print any routes. Printing of the routes is done by the solid-start cli on the start command by using the router. This requires vite etc. things not needed in production. This is why I refer to the start command as preview build.

So you are saying, that the server works just fine, but it simply doesn't print out the routes?

edivados commented 1 year ago

Yes, exactly.

devinrhode2 commented 1 year ago

I think a pre-requisite for this issue would be to actually establish a contract for solid.js deployments here: https://github.com/solidjs/solid-start/discussions/1005

I suppose we could add e2e tests for various prod-deployment scenarios in the future, which verify that pnpm, npm and yarn all work for the production deployment contract/documentation we lay out. I suppose this will basically just be a series of shell scripts. Here's an example for npm:

npm init solid@latest
rm -rf node_modules
npm ci --omit=dev # not sure what actual contract is
npm run build # we could add snapshots on the output here, which auto-update?
npm run start # # could also add snapshots here, which also auto-update
(Wait 5 seconds, try loading a page?, verify no console errors?)

Again I think a portable package.json would be best. It should be portable across latest versions of all major package managers. For support of older package managers, users will have to use older versions of solid.

Closing this for now, as it's just not very important imo