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

Asset pipeline upgrade #1025

Open
texpert opened this issue Dec 14, 2022 · 7 comments
Open

Asset pipeline upgrade #1025

texpert opened this issue Dec 14, 2022 · 7 comments

Comments

@texpert
Copy link
Collaborator

texpert commented Dec 14, 2022

From this post

The default (without node/yarn)

    Rails 7: Import Maps + Sprockets
    Rails 8: Import Maps + Propshaft

The “choose your own bundler” (with node/yarn)

    Rails 7: Bundling Gems + Sprockets
    Rails 8: Bundling Gems + Propshaft

First, default option, is best, but it works only with ES6 JS modules.

Which leaves us for now considering the 2nd option - using the jsbundling-rails and cssbundling-rails gems with Sprockets (migration to Propshaft may come later). Still not sure if it will work with CJS, needs testing.

And the main hitch is that no bundling solution has developed a workflow for working with JS assets from the gems/engines. There was a certain guide for Webpacker in a now deleted commit, but it is so cumbersome (even ugly), requiring running additional Webpacker instances per each added engine, that is a no go.

So we need an additional GitHub repo for the JS package.

WDYT, @brian-kephart, @owen2345?

@brian-kephart
Copy link
Collaborator

Definitely no webpacker. It never had a good story for engine assets, and now that it's deprecated there's no need to support it.

It sounds like we can more or less count on any Rails app having either Sprockets or Propshaft. My understanding of Propshaft is that it's like Sprockets but with fewer features, so is it the case that if we target Propshaft then things should continue to work with Sprockets as well?

The bundling gems work by feeding the output of the bundlers into Sprockets/Propshaft, so if our assets are available to Sprockets/Propshaft directly then the bundlers themselves are irrelevant to us. I think this is more or less the case for import maps as well, though we should definitely test that. For this reason, I do not think we need a separate NodeJS package. Please let me know if I'm missing something here.

Overall, it seems like we just need to make sure that Camaleon works with Propshaft. It looks like that means converting SASS to CSS and CoffeeScript to JS. I've actually tried to do this before to remove the sass-rails dependency, but something always gets the better of me. I would LOVE it if we could eliminate these dependencies. I see this as an opportunity to make our assets simpler, rather than more cumbersome.

The process I would propose:

  1. Make a new branch for experimental Propshaft support.
  2. Remove the sprockets-rails dependency.
  3. Migrate all CoffeeScript to JS and remove the coffee-rails dependency.
  4. Migrate all SASS to CSS and remove the sass-rails dependency.
  5. Add some Propshaft builds to Github Actions.
  6. If all goes well, merge the branch and release a v3 with the changes.

Do you see any problems with this approach?

@texpert
Copy link
Collaborator Author

texpert commented Dec 15, 2022

Not quite so.

The jssbundling-rails is feeding the JS assets to either Webpack, or Rollup, or Esbuild.

"Use esbuild, rollup.js, or Webpack to bundle your JavaScript, then deliver it via the asset pipeline in Rails"

The reason for having a JS package is that we don't use only ES modules JS and because of this we couldn't use importmaps yet - see DHH's comment on the issue of building JS assets from engines

Esbuild is known to having some issues, and from Webpack and Rollup I'd choose Webpack as Rollup is supporting only ES modules and is less universal.

As for SASS vs CSS, SASS has become a standard - see dartsass. And dartsass is supported by the cssbundling-rails and propshaft - see rails/propshaft#92.

So one thing which certainly must be done first is:

  1. Migrate all CoffeeScript to JS and remove the coffee-rails dependency.
  • Test thoroughly and craft a new minor release.

Then we have 2 choices to think on first:

  • either upgrade all the JS dependencies one by one to modern versions having ES modules and then migrate to importmaps
  • or craft a JS package and start using jsbundling-rails with Webpack, and then start upgrading the JS dependencies one by one

The second option is tougher to implement, but it seems to give more flexibility on the JS upgrade path to ES modules. It could be a long way - upgrading all the JS to ES modules.

Not quite sure when the Sprockets to Propshaft + dartsass migration should occur - seems that it could be tried any time.

@texpert
Copy link
Collaborator Author

texpert commented Dec 19, 2022

BTW, jQuery isn't yet ESM compatible - they are planning it only for the upcoming 4.0 release. See jquery/jquery#4592

@brian-kephart
Copy link
Collaborator

Looks like Propshaft compatibility will become relevant with the release of Rails 8 this year.

@brian-kephart
Copy link
Collaborator

Thinking about this some more, I think that we need to build assets at release time instead of relying on the end user's build path.

For example:

# gemspec
s.add_development_dependency 'jsbundling-rails'
s.add_development_dependency 'cssbundling-rails'
# bin/build_release
#!/usr/bin/env ruby

system "some command to clear /specified/asset/directory"

# these commands would build in /specified/asset/directory
system "yarn build"
system "yarn build:css"

system "gem build" # gem will be built with the bundled asset files

That way, we can choose whatever build pipeline we want, and we don't have to support the complications of the end user's build pipeline or require them to use anything specific. We just have to leave the pre-built undigested files where Propshaft can find them (because if Propshaft can find them, so can Sprockets). If handled carefully, this might not even result in a breaking change.

Thoughts?

@texpert
Copy link
Collaborator Author

texpert commented Jan 4, 2024

I am more and more inclined to the upgrade all the JS dependencies one by one to modern versions having ES modules and then migrate to importmaps way.

@brian-kephart
Copy link
Collaborator

That would be ideal, but I thought some of the dependencies don't have ESM compatible versions. I'd love to be mistaken about that, though.

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

No branches or pull requests

2 participants