search

srcbookdev / srcbook

TypeScript-centric app development platform
https://srcbook.com
Apache License 2.0
2.26k stars 70 forks source link

Add an explicit "npm install" loading state, and start to gate features behind if node_modules exists #389

Closed 1egoman closed 2 weeks ago

1egoman commented 2 weeks ago

This pull request does a few things:

  1. Implements (mostly) the package installation states in figma.
  2. Adds a new deps:clean action that allows the app to remove the node_modules directory a. I introduced this largely for my own testing. If this isn't actually a good idea to have in the app long term, I can remove it.
  3. Adds a new deps:status and deps:status:response action that allows the app to get information about the installed dependencies. Right now this returns an object with a nodeModulesExists key, which is used by the app to determine if the user has installed dependencies yet.
  4. Makes some adjustments to the preview tab to take into account this new installation state. Namely, instead of auto installing dependencies like before, it now blocks and the user is expected to click the "install" button in the new popup in the lower left corner.

Note that this still doesn't rename settings to packages or refine the panel's interface - I ran out of time today. I will get to that on monday! There's also some additional interface guarding work left.

https://github.com/user-attachments/assets/609befb0-b448-4a1a-941c-ddd07ed5a4f1

1egoman commented 2 weeks ago

I also want to call out: I've noticed a lot of instability in the logic that kicks off running vite. It seems like sometimes it locks up / becomes unresponsive, and I'm not 100% sure why. I don't think it's related to something I've introduced in this change as it also seems to happen on main.

My best lead is that in packages/api/package.json, dev is set to vite-node -w ... - that -w causes node to get live reloaded. Maybe srcbook isn't properly killing all of it's subprocesses? More investigation needs to be done here.

nichochar commented 2 weeks ago

"Packages need to be installed" is a subtle affair, we did some work for that in the original notebook. This PR introduces the high level "does node_modules exist check. Worth noting that we have a library and some utils in there called depcheck that you can run and it will tell you if packages need to be installed. It is slightly smarter as it knows to check for the state where some libraries are installed, and node_modules exists, but the package.json listed deps are not all there (using package-lock as a source of truth).

I'm bringing this up because we might want different levels of checks. I think checking for node_modules and blocking the server preview is a solid first pass though, and so this is something we can build on.