solidus icon indicating copy to clipboard operation
solidus copied to clipboard

app/overrides files loading twice

Open diegomichel opened this issue 2 years ago • 5 comments

Hello, it looks like files under app/overrides are loading twice when following instructions from: https://guides.solidus.io/customization/customizing-the-core/#using-overrides:~:text=config/application.rb

I noticed this issue because I decided to add a constant in one of the overrides and was getting warnings from ruby; besides the warnings, it doesn't seem to cause any other issues.

deface gem seems to be loading the files without a need to change the config/application.rb. deface is not a direct dependency of solidus but comes with recommended gems solidus_auth_devise and solidus_paypal_commerce_platform Solidus Version:

3.3.0

To Reproduce Clone the following repo: https://github.com/diegomichel/solidus-override-issue

Run bundle exec rails c. You will see a message printed twice as defined on app/overrides/amazing_store/spree/product/add_global_hidden_flag.rb

Current behavior App loads app/override files twice

Expected behavior To load app/override files once

Screenshots Screenshot 2023-01-25 at 16 07 25 Screenshot 2023-01-25 at 16 07 35

caller diff from first and second time it loads Screenshot 2023-01-25 at 15 41 56

diegomichel avatar Jan 25 '23 23:01 diegomichel

Thanks for reporting, @diegomichel. Yeah, that's a known problem, but it's nice you created a dedicated issue as it'll have more visibility. See https://github.com/solidusio/solidus/pull/4231:

On a related information, the recommended app/overrides/ directory is the same used by deface (https://github.com/spree/deface), a common dependency in Solidus projects. However, both types of overrides can coexist without interfering with each other (see https://github.com/solidusio/solidus/issues/3010#issuecomment-844252252). We decided to tackle the issue upstream on deface (see https://github.com/solidusio/solidus/issues/3010#issuecomment-845034428).

And, from the second referenced comment:

For the time being, I think that we should allow Deface to accept the overrides path as a configuration option, and push to deprecate and then remove the default setting of app/overrides, so that it doesn't clash with Rails overrides. A couple of us are also Deface maintainers, and I'm sure we can work with our Spree friends to get this done, since it's in the best interest of both platforms.

However, the work hasn't been done in Deface, yet.

You can work around by using a different directory for your own overrides, like app/monkey_patches/.

waiting-for-dev avatar Jan 26 '23 05:01 waiting-for-dev

Thanks so much 🙏 for the write-up and the workaround, @waiting-for-dev; it looks like you guys went over all of this.

diegomichel avatar Jan 26 '23 14:01 diegomichel