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

Remove sassc-rails dependency when using webpacker #1617

Closed
adipasquale opened this issue Apr 20, 2020 · 15 comments
Closed

Remove sassc-rails dependency when using webpacker #1617

adipasquale opened this issue Apr 20, 2020 · 15 comments
Labels
frontend-pipeline Webpacker vs Sprockets

Comments

@adipasquale
Copy link

Hi! I'm wondering how hard it would be to remove sassc-rails as a dependency?

It becomes useless when you're using the full-blown webpacker way to do things (which is by the way very well documented in the wiki, thank you).

I'm guessing it should be left up to the user's Gemfile to decide whether they want to use sassc-rails or webpacker.

Let me know if that's one way you'd like to go and I can try patching a version without sassc-rails.

@pablobm
Copy link
Collaborator

pablobm commented Apr 21, 2020

I'm not sure of what the consequences of this are. Here's a related conversation that illustrates a difference between the asset pipeline and webpacker: #1617.

I don't know the pipeline or webpacker well enough to make an informed decision right now. However I agree that we'd eventually want to sort this question out in order to improve the user experience.

Before we change anything, we'll have to resolve some questions. The first ones that spring to mind are: if a user is using webpacker, how would their experience be affected if we removed references to the asset pipeline? And the other way around? If there are any issues, how can these be addressed?

@adipasquale
Copy link
Author

hey thanks for the quick answer! I think you made a mistake and pointed at this current ticket rather than another one.
I am far from being an expert on these topics either, but I'd be willing to try and see what happens on my end if I remove references to the asset pipeline from administrate and go full-webpacker.

@pablobm
Copy link
Collaborator

pablobm commented Apr 21, 2020

Ooops, you are right. The correct link is #1609.

If you want to try experiment that would be great! The more input we have, the better we'll be able to understand the issue.

@pablobm pablobm added the assets label Apr 23, 2020
@nickcharlton nickcharlton added views-and-styles how administrate looks and is interacted with and removed assets labels Apr 29, 2020
@nickcharlton
Copy link
Member

I think we've got a path here that's either:

  1. Only support Webpacker for assets (which does seem to be the Rails future),
  2. Allow using both, switching on Rails versions (or, better, known presence of Webpacker?)

This would wrap this issue and others like #1609.

@adipasquale
Copy link
Author

@nickcharlton do you think that option 1 is on the table? I believe many people don't use webpacker yet. Doing this would limit Administrate's support quite drastically don't you think?

@nickcharlton
Copy link
Member

I think it's hard to be confident enough to force a switch to Webpacker without any data on actual usage and we won't be able to get that.

Some variation on 2 is the right approach and what I'd like to do.

@adipasquale
Copy link
Author

adipasquale commented May 3, 2020

I had a look into it today. My plan was to simply move the sassc-rails dependency from Administrate's gemspec file to the host app's Gemfile and see what happens : cf https://github.com/adipasquale/administrate/commit/40b00b72e4f27784ac831ffc2f08caf84f5e7817. This breaks the Engine entirely.

I have 0 experience with Rails Engines. There is no straightforward way to use webpacker inside an engine (cf this issue: rails/webpacker#348), let alone making an engine work hybridly with both sprockets and webpacker.

The option I'm seeing would be to check for sprocket gem presence in the Engine and conditionally perform assets-related tasks there. Do you think it's doable and a good direction? I'll continue digging into that direction if you think it's a good idea!

@adipasquale
Copy link
Author

adipasquale commented May 3, 2020

I tried looking at other similar rails gems to see how they handle this.

Both ActiveAdmin and rails_admin seem to have very different engine files. They use initializers to add precompile paths for their assets:

ActiveAdmin has a use_webpack flag that makes it insert javascript_pack_tag instead of javascript_include_tag: activeadmin/activeadmin@d6f240a

@pablobm
Copy link
Collaborator

pablobm commented May 8, 2020

We may have to create a setting like ActiveAdmin's use_webpack, as I don't see that there's a sure-fire way to detect that the user is using one or the other, particularly as both can be active at the same time.

It would be a bit of a bummer, since we have so far managed not to have any such app-wide configuration.

@adipasquale - Would you be able to put together a PR, so that we can start testing potential issues?

@nacengineer
Copy link
Contributor

nacengineer commented Jun 10, 2020

I think the whole industry is going to be on webpacker when all is said and done. It makes sense since JS has (IMO) inconsistent patterns to include. Webpacker smooths that and makes the JS bits follow a JS pattern. It will be a pain point for certain.

The webpacker option IMO would be to create an parallel npm package for delivery of the js/css/image assets. This fits with how webpacker works. It could be similar to how foundation and bootstrap projects are doing it. I think that's a decent model to mimic.

Then the require can happen via webpacker file and this gem only has to deliver the rails bits.

@pablobm pablobm added the frontend-pipeline Webpacker vs Sprockets label Jun 11, 2020
@nickcharlton
Copy link
Member

I think the whole industry is going to be on webpacker when all is said and done.

Yeah, I agree and that's how myself and thoughtbot as a whole are approaching new projects. It's a bit different for Administrate and we're going to have a (long) period of a solution of both, which is a pain but unavoidable.

The webpacker option IMO would be to create an parallel npm package for delivery of the js/css/image assets

Ahh yes, of course. I've seen this. That makes a lot of sense.

@sfcgeorge
Copy link

We use Administrate with Webpacker and no Sprockets (as per the wiki). Having sassc automatically installed is a bit annoying because it's slow and Rails builds are frustratingly slow (especially in Docker) at the best of times so any speed up is welcome 😉

The simplest way I think is @adipasquale's suggestion of just removing the dependency from the gemspec. I believe the reason it didn't work for them is because you don't add it to Administrate's Gemfile, you add it to your application's Gemfile. Rails/Bundler installs gems from Engines' gemspecs but not from their Gemfiles (plus we're trying to remove the automatic dependency anyway).

Anyone who is actually using Sprockets will already have the sassc gem by default, but just in case you can add to the README "Please make sure you have the sassc gem in your Gemfile (unless you're using the manual Webpacker approach [[link to wiki]])". Anyone using Webpacker will already be used to the nightmare of configuring Webpacker so won't mind that it isn't automatic here 😄

Bundler Gemspecs can't execute conditional code on install unfortunately; there's no way to check the environment and selectively install things. So while making Administrate work with Webpacker out of the box would be lovely it wouldn't actually solve this sassc dependency issue. So I'd simply:

  • Remove dependency from gemspec
  • Add line to README

And have a separate issue for built in Webpack(er) support if anyone wants to maintain that.

@nacengineer
Copy link
Contributor

nacengineer commented Oct 22, 2020

because it's slow and Rails builds are frustratingly slow (especially in Docker)

FWIW try to do a multi-stage build where you install the sassc gem manually in the first stage. This should cache that gem install and subsequent builds should be faster. I do this for nokogiri too as that one is a long install also.

@nickcharlton
Copy link
Member

I think @sfcgeorge's idea of going ahead and removing it is a good idea, all things considered. Would anyone be able to open a PR for that?

I would like to see some sort of hook/full support for either Sprockets or Webpacker long-term, but prior to that, reducing the surface area we need to care about is valuable on it's own.

@nickcharlton nickcharlton mentioned this issue Jan 20, 2023
3 tasks
@pablobm pablobm removed the views-and-styles how administrate looks and is interacted with label Apr 19, 2023
@nickcharlton
Copy link
Member

I'm going to close this because we've just merged in #2397, which will mean in our next release we'll be bundling the assets required for Administrate in the gem itself.

I'm going to start releasing some pre-release versions of the gem in the coming days, as I'm sure I'm about to break stuff for people, and so we can get feedback and make sure there's nice migration help sorted out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend-pipeline Webpacker vs Sprockets
Projects
None yet
Development

No branches or pull requests

5 participants