Skip to content
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

Reduce/eliminate our Node use #188

Closed
reefdog opened this issue Apr 21, 2022 · 3 comments
Closed

Reduce/eliminate our Node use #188

reefdog opened this issue Apr 21, 2022 · 3 comments
Assignees
Labels
Ice Box For features to come

Comments

@reefdog
Copy link
Contributor

reefdog commented Apr 21, 2022

After some Slack chatter with @cguess, it's… possible that we don't actually need Node/yarn on this project at all.

  • Rails 7 defaults to a Webpack-free, Node-less system using importmap.
  • The libraries we're using that used to be delivered via npm packages (Turbo, Stimulus, Tailwind) are provided via gems now.

This issue is for discussing (if necessary) any consequences, tracking any complications, and then actually doing it.

Among the unknowns:

  • How much will we regret only having access to the npm ecosystem through the importmap pinning? E.g., we can no longer patch packages to resolve issues like this one with Flowbite+Turbo (unless we do something real old-school like just include the package in a vendor/ dir); but maybe those are dangerous anti-patterns anyway?
  • ESLint: Maybe this is the single thing we keep Node around for? Reducing our surface area to just dev dependencies would still be a win.
@reefdog
Copy link
Contributor Author

reefdog commented Apr 21, 2022

Once we've done this, I think we can also remove the cssbundling-rails gem. See this FAQ note.

Edit: Removed this gem as part of #189 for reasons explained in its description.

@reefdog
Copy link
Contributor Author

reefdog commented Apr 25, 2022

I'm not going to rush this because there are more important things to do (although this is important to tackle soon so that our codebase has a very clear MO), but I did just do a quick test of the following changes, and it all worked just fine:

  • Primary: Delete package.json, yarn.lock, and node_modules/
  • Cleanup:
    • Remove the Node reference from .tool-versions
    • Delete babel.config.js, postcss.config.js, and tsconfig.json

After doing that, the app still ran and worked just fine. What I suspect we'd also have to do to close the loop:

  • Update documentation
  • Delete Node details from Heroku's app.json
  • Delete Node details from Docker's docker-compose.yml and Dockerfile (not that we're using it)

@reefdog
Copy link
Contributor Author

reefdog commented Apr 26, 2022

Well, apologies for the whiplash, but I no longer thing we should do this, and in fact think we should revert back entirely to a bundled setup.

The pragmatic justification is that I think we should use Flowbite, and we simply can't use Flowbite correctly with importmaps. Full stop. DHH says as much here.

  • The small/resolvable blocker is that we currently have to patch Flowbite in order to get it to work alongside Turbo. This can be automated in the Node world, but in the importmap world requires vendoring flowbite.js and updating it manually after each update.
  • The larger/unresolvable blocker, though, is that to use Flowbite, we have to extend Tailwind with the Flowbite "plugin" (see step 2 here), and that's just not doable without a build step.

The philosophical justification uses the pragmatic one as a case study in how complicated and limiting it (currently) is to live without the normal expected access to the npm ecosystem; we can use a lot of it, but with exceptions and customizations and contortions that complicate life. If we were only writing our own JS, or knew we'd only be using individual/atomic JS libraries, then importmaps would be great. And I hope we can convert back at some point! But for now, I think it's too early / not advantageous enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ice Box For features to come
Projects
None yet
Development

No branches or pull requests

1 participant