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

Custom Acronym for UI breaks other gems/engines #6075

Open
brandonjyuhas opened this issue Jan 15, 2025 · 7 comments
Open

Custom Acronym for UI breaks other gems/engines #6075

brandonjyuhas opened this issue Jan 15, 2025 · 7 comments

Comments

@brandonjyuhas
Copy link

Solidus has a custom inflection acronym for "Ui" (changing it to "UI"). This is correct in how UI is used. However, this means that other engines which use the default inflection ("Ui") are broken by default.

This means that to use another gem (such as Maglev), you need to monkeypatch their gem to rename the module.

Defining the inflection of a commonly used acronym within in apps codebase seems entirely outside of the scope of what Solidus provides, and I should be able to use Solidus without it changing the names of constants coming from other gems.

Solidus Version:
4.4.0

To Reproduce

Define a module using the default inflection for ui, "Ui". Alternatively, use a gem (such as Maglev). Note that it cannot be included in any class.

Current behavior
Constants including the default inflection for ui cannot be loaded.

Expected behavior
The default inflection for 'ui' is respected.

Screenshots

Image

Desktop (please complete the following information):

  • OS: OSX
  • Browser: Chrome
  • Version 131.0.6778.265 (Official Build) (arm64)
@fthobe
Copy link
Contributor

fthobe commented Jan 31, 2025

@elia will have to tell you if this needs to be or if it was only a quality of life change 5e719db

@fthobe
Copy link
Contributor

fthobe commented Feb 8, 2025

@rainerdema was this a quality of life change or something that was inevitable?

@elia
Copy link
Member

elia commented Feb 10, 2025

This is a pesky issue, but it's a sensible request.
I wonder if there's a way to inflect UI for a specific namespace, otherwise makes total sense to just go with the default inflection.

@fthobe
Copy link
Contributor

fthobe commented Feb 10, 2025

@elia Would it break anything significant to rework that change?

The commit says:

Add an inflection to consider "UI" as an acronym
The DRY.rb container needs to be connected manually to Rails
inflections otherwise it will use its own and render "UI" as "Ui".

Co-Authored-By: Rainer Dema <rainer.dema@gmail.com>

but I am having a hard time to understand the full impact here.

@elia
Copy link
Member

elia commented Feb 10, 2025

@fthobe
@brandonjyuhas
would any of you be open to look into https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#customizing-inflections and send a PR fixing it?

I think we can and should strive to keep UI for solidus without disrupting the outside world.

@fthobe
Copy link
Contributor

fthobe commented Feb 10, 2025

@elia and just like that Elia brought ravage to another community unprepared for @fthobe 💃

@fthobe
Copy link
Contributor

fthobe commented Feb 10, 2025

@jhawthorn @amatsuda do you think it makes sense to touch this? We are unsure if we should align with rails or rails or rails should align with us here.

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

3 participants