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

app/overrides files loading twice #4877

Open
diegomichel opened this issue Jan 25, 2023 · 2 comments
Open

app/overrides files loading twice #4877

diegomichel opened this issue Jan 25, 2023 · 2 comments
Labels
confirmed Validated report good first issue Wanted, but lacking time to work on it and might be a good fit for a first contribution type:bug Error, flaw or fault

Comments

@diegomichel
Copy link

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

@waiting-for-dev
Copy link
Contributor

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 #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
#3010 (comment)).
We decided to tackle the issue upstream on deface (see
#3010 (comment)).

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 waiting-for-dev added confirmed Validated report type:bug Error, flaw or fault good first issue Wanted, but lacking time to work on it and might be a good fit for a first contribution labels Jan 26, 2023
@diegomichel
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed Validated report good first issue Wanted, but lacking time to work on it and might be a good fit for a first contribution type:bug Error, flaw or fault
Projects
None yet
Development

No branches or pull requests

2 participants