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

Fix setting custom preview_paths #2182

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tvdeyen
Copy link

@tvdeyen tvdeyen commented Dec 16, 2024

Setting preview_paths to anything else than test/components/previews is broken, because we mutate ActiveSupport::Dependencies.autoload_paths.

From the Rails guides:

Please do not mutate ActiveSupport::Dependencies.autoload_paths;
the public interface to change autoload paths is config.autoload_paths.

Fixes #365

@Spone
Copy link
Collaborator

Spone commented Dec 16, 2024

Wow thanks for this! Any idea why the CI is failing?

EDIT: also, can you add a changelog entry for this?

@tvdeyen
Copy link
Author

tvdeyen commented Dec 16, 2024

Wow thanks for this! Any idea why the CI is failing?

No. It seems it is broken for a long time https://github.com/ViewComponent/view_component/actions/workflows/ci.yml
Probably the Rails 8.0 release? Since the main branch is not building it seems very unlikely that the issues are related with this change tbh. I recommend that a maintainer fixes the issues on current main first.

EDIT: also, can you add a changelog entry for this?

Sure. Can do.

@tvdeyen tvdeyen force-pushed the fix-auto_load_paths branch from 8be8086 to 092dda3 Compare December 16, 2024 15:30
@tvdeyen
Copy link
Author

tvdeyen commented Dec 16, 2024

@Spone so, specs seem to fail because of #1659

action_view loaded too soon

is the error and then it exits. I assume that due to the fact that we now add preview_paths to autoload_paths way earlier (before :set_autoload_paths from Rails). My guess is that this loads ActionView::Base somehow.

We need to be that early though, because autoload_paths is frozen after :set_autoload_paths which causes #365

I don't know which problem #1659 solves. Maybe @camertron can shed some light?

Setting `preview_paths` to anything else than `test/components/previews`
is broken, because we mutate `ActiveSupport::Dependencies.autoload_paths`.

From the Rails guides:

> Please do not mutate ActiveSupport::Dependencies.autoload_paths;
> the public interface to change autoload paths is config.autoload_paths.

Fixes ViewComponent#365
@tvdeyen tvdeyen force-pushed the fix-auto_load_paths branch from 092dda3 to 432e826 Compare December 17, 2024 07:47
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.

Can't modify frozen autoload_paths in development environment
2 participants