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

The sprockets version requirement doesn't really lock to v3 #3452

Closed
elia opened this issue Dec 10, 2019 · 3 comments
Closed

The sprockets version requirement doesn't really lock to v3 #3452

elia opened this issue Dec 10, 2019 · 3 comments

Comments

@elia
Copy link
Member

elia commented Dec 10, 2019

Sprockets requires version ~> 3 in the main Gemfile wich is actually the same as >= 3, the inteded requirement would be ~> 3.0.

I also didn't see sprockets among dependencies of either solidus_fronted or solidus_backend, both of them are actually using it and look like they should explicitly depend on it, but maybe there's a good reason not to do it.

Linked issues: #3374, #618, #3378, #3373

Solidus Version:

All supported versions should update the dependency until Sprockets 4 is properly supported

@kennyadsl
Copy link
Member

You are right @elia, we should change that. I think a new release is not needed here since the Gemfile is used in Solidus development only and will not be useful to compute dependencies in apps that are using Solidus.


I also didn't see sprockets among dependencies of either solidus_fronted or solidus_backend, both of them are actually using it and look like they should explicitly depend on it, but maybe there's a good reason not to do it.

Interesting, that's the reason why sprockets is not actually locked to 3.x. I can't find any reasons for that, I think Solidus backend can't work without sprockets at the moment and we should add the dependency. For the frontend, as is, it could be a useless dependency for who is using Webpack for everything (JS, CSS, and images).

I'd say we could add it to backend only since I don't know of any store that is using the Solidus frontend without the backend as well. Thoughts?

At this point, changing the gemspec we'll need a new release.

@kennyadsl
Copy link
Member

Thinking twice, I think we are supporting Spockets 4 when Solidus is mounted on a Rails application since it should have the assets configuration manifest in place, as Sprockets 4 expects.

What we are not having, is only the support for the Dummy app that we use in our test suite, since there's no way to set a custom path for that file, see rails/sprockets-rails#446.

Which issue are you having exactly @elia? Cause if sprockets were not locked to v3 and we were using v4, I can't figure out how specs are passing in this period...

@elia
Copy link
Member Author

elia commented Dec 11, 2019

Thanks for looking into this @kennyadsl, I actually misinterpreted the problem solved by the restriction on the sprockets version within Solidus, I though it wasn't compatible at all with Sprockets 4. As of the ~> 3 requirement apparently there's an exception to the rule that will make it behave just like ~> 3.0, so we're good on that side too. 👍

I would add that might make sense to add sprockets-rails as a backend/frontend dependency, but it's also a reasonable expectation that anyone not using the default rails stack also know what they're doinig.

@elia elia closed this as completed Dec 11, 2019
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