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

Re-enable flickwerk-style patches #94

Merged
merged 4 commits into from
Jan 24, 2025

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Dec 18, 2024

Summary

We found out that we need to find and load patches in a config.to_prepare hook - the hard way, because Flickwerk 0.2 broke quite a few other gems and extensions.

The new release has that fixes and offers a new API for class aliases like "Spree.user_class" => "Spree::User".

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

@mamhoff mamhoff changed the title Re enable flickwerk Re-enable flickwerk-style patches Dec 18, 2024
@mamhoff mamhoff force-pushed the re-enable-flickwerk branch from f64be5e to 6786a9b Compare December 18, 2024 17:58
Rather than iterating over all patches, we can tell Flickwerk upfront
what to replace.

Update Flickwerk dependency to 0.3.4

This should not mess with load order the way 0.2 did.

Add patch paths in `lib` to autoload paths.

Patches should be autoloaded, and this will do it.

Load User class aliases in app reloader prep callback

We need to delay setting the alias of `Spree.user_class` to the
configured `Spree.user_class_name` to just before loading the patches,
otherwise we end up with the wrong user class name in some cases.
We want our initializers to be calles something like

`solidus_reviews_backend_paths`

rather than

`Solidus::Reviews_backend_paths`,

because the latter looks odd.
@mamhoff mamhoff force-pushed the re-enable-flickwerk branch from 3c30516 to e425951 Compare January 24, 2025 15:53
Solidus versions < 4.5 `require` some of their application code.
This leads to hard-to-debug problems with Flickwerk patches.
What this does is eager-load all the patches in a `to_prepare`
hook by constantizing them.
@mamhoff mamhoff force-pushed the re-enable-flickwerk branch from e425951 to eec7710 Compare January 24, 2025 15:54
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work. We tested it in several extensions. This is ready to go 🎉

@tvdeyen tvdeyen merged commit e39017c into solidusio:main Jan 24, 2025
3 checks passed
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

Successfully merging this pull request may close these issues.

3 participants