Skip to content

Conversation

@guilleiguaran
Copy link
Member

assets:precompile task is not being enhanced and therefore not executing
webpacker:compile when referencing webpacker master branch on Gemfile,
reverting back to Engine fix the problem immediately.

This reverts commit 6586ef7 and 2c0c7dc.

assets:precompile task is not being enhanced and therefore not executing
webpacker:compile when referencing webpacker master branch on Gemfile,
reverting back to Engine fix the problem immediately.

This reverts commit 6586ef7 and 2c0c7dc.
@rafaelfranca
Copy link
Member

How about fixing the loading of the rake task? rake_tasks is a valid method on railtie. The test_unit railtie uses it for example to add rake test.

@guilleiguaran
Copy link
Member Author

guilleiguaran commented Oct 31, 2017

Strangely assets:precompile task isn't enhanced when referencing master:

gem "webpacker", git: "https://github.com/rails/webpacker.git"

but cloning webpacker to a local folder and pointing to it in Gemfile works as expected and assets:precompile is enhanced:

gem "webpacker", path: "~/src/webpacker"

Moving Webpacker to an Engine makes the first work, I'll continue researching a bit more.

@guilleiguaran
Copy link
Member Author

guilleiguaran commented Oct 31, 2017

Found something, adding explicitly to sprockets-rails on Gemfile before of webpacker makes this work:

This doesn't work:

gem "webpacker", git: "https://github.com/rails/webpacker.git"
gem "sprockets-rails"

This work:

gem "sprockets-rails"
gem "webpacker", git: "https://github.com/rails/webpacker.git"

And again changing Webpacker to an Engine makes this work regardless of the order.

This work:

gem "webpacker", git: "https://github.com/rails/webpacker.git", branch: "revert-to-engine"
gem "sprockets-rails"

@guilleiguaran
Copy link
Member Author

Note that in all cases the Webpacker rake tasks are correctly loaded is just enhancing that doesn't work because Rake::Task.task_defined?("assets:precompile") returns false in the failing cases.

@matthewd
Copy link
Member

As a matter of principle it does feel slightly more correct for this to be a Railtie instead of an Engine. I don't feel strongly about that, but do worry about whether this Railtie-vs-Engine difference is by design, or is just luck that it ends up running at a 'better' time.

However we decide to fix it, it sounds like we need a test to prove that the task gets enhanced regardless of the load order.

@rafaelfranca
Copy link
Member

The difference between an engine and railtie regardless the load order is:

The railtie executes the rake_tasks blocks in order of definition.
The engine loads the lib/tasks files after all railties are loaded.

I don't think it is by design. It looks like an implementation detail.

I also thing an Railtie is more correct, but I don't have strong opinion either.

@gauravtiwari
Copy link
Member

@guilleiguaran Guess we fixed this with rake_tasks method on railtie right? I can't reproduce this on master.

@guilleiguaran
Copy link
Member Author

@gauravtiwari I'm going to test this again and report results here

@gauravtiwari
Copy link
Member

Sure 👍 thanks

@gauravtiwari
Copy link
Member

@guilleiguaran Were you able to test this one? Thinking to make a new release this weekend.

@guilleiguaran
Copy link
Member Author

guilleiguaran commented Dec 10, 2017

I can reproduce this as described in #985 (comment) and it still being fixed immediately switching to revert-to-engine branch.

I'll merge this for now to unblock the release and check if we can do something else about the loading order issue.

@guilleiguaran guilleiguaran merged commit 3e738c4 into master Dec 10, 2017
@guilleiguaran guilleiguaran deleted the revert-to-engine branch December 10, 2017 00:08
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.

5 participants