read webpacker config to populate autoload paths #36803
Conversation
andrewkress
force-pushed
the
fix-issue-36799
branch
4 times, most recently
from
26fe16c
to
eed744c
Compare
andrewkress
changed the title
andrewkress
force-pushed
the
fix-issue-36799
branch
from
eed744c
to
c3f2524
Compare
There was a problem hiding this comment.
LGTM. I am not sure about the rescue part. Do we need to rescue so many things? And I think since we are doing early return rescue won't be necessary at all(I haven't checked this).
You might need to squash your commits in one. Thanks. Over to the team members now 😄
|
Probably don’t need to rescue with the if statement put back in there. I can remove those and squash. |
andrewkress
force-pushed
the
fix-issue-36799
branch
2 times, most recently
from
cdf885e
to
fe2e9d3
Compare
|
added the rescue LoadError back in, because if the webpacker.yml file exists, but the gem has been removed, there will be a: |
andrewkress
force-pushed
the
fix-issue-36799
branch
from
fe2e9d3
to
913d39c
Compare
andrewkress
force-pushed
the
fix-issue-36799
branch
from
913d39c
to
ffaee47
Compare
|
|
||
| ActiveSupport::Dependencies.autoload_paths.each do |path| | ||
| assert_not_operator path, :ends_with?, "app/assets" | ||
| assert_not_operator path, :ends_with?, "app/webpack" |
There was a problem hiding this comment.
How is this test passing? We are not setting the config to webpack anywhere yet. Unless I'm missing something.
There was a problem hiding this comment.
I added a new commit to this branch that makes this test fail if the config is not set.
read webpacker config to populate autoload paths
|
Thank you for working on this! |
Summary
this change fixes the issue reported in #36799 when an environment is not in webpacker.yml. It will default to look in the production configuration.
Other Information