Allow setting REACT_ON_RAILS_PRECOMPILE to false to skip building #1371
Conversation
|
@Judahmeek can you finish up this one? |
Judahmeek
force-pushed
the
allow-skip-custom-compile
branch
from
5aa50ab
to
c55abfe
Compare
|
Rebased & linted |
Judahmeek
force-pushed
the
allow-skip-custom-compile
branch
from
c55abfe
to
9a5b7c5
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @justin808)
lib/tasks/assets.rake, line 10 at r3 (raw file):
ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development" skip_react_on_rails_precompile = %w[no false n f].include?(ENV["REACT_ON_RAILS_PRECOMPILE"])
Hi @Judahmeek, we need documentation and a changelog entry for this one. I'll hold off on merging so we don't forget.
Judahmeek
force-pushed
the
allow-skip-custom-compile
branch
from
1dab8ad
to
8a9739e
Compare
| ## In the project including your engine | ||
|
|
||
| Place `gem 'react_on_rails', '~> 6'` before the gem pointing at your engine in your gemfile. | ||
|
|
||
| This is necessary because React on Rails attaches itself to the rake assets:precompile task. It then uses a direct path to cd into client, which will not exist in the main app that includes your engine. Since you'll always be precompiling assets in the parent app, this will always fail. The workaround then, is to remove the task and replace it with one that goes into your Engine's root. The reason you have to include the react on rails gem before your engine is so that the `react_on_rails:assets:compile_environment` task is defined by the time your engine gets loaded to remove it. | ||
|
|
There was a problem hiding this comment.
Found this while looking for where assets:precompile was mentioned.
Removed this because it's out of date: the task is assets:webpack instead of assets:compile_environment & cding to client is no longer hard coded. prepend_cd_node_modules_directory cd's to node_modules_location, which is Rails.root by default.
I can move the changes to this file to another PR if you'd prefer.
Judahmeek
force-pushed
the
allow-skip-custom-compile
branch
from
4072c66
to
678f428
Compare
There was a problem hiding this comment.
This PR is fine.
small comments...
Reviewed 4 of 4 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @justin808)
docs/guides/how-react-on-rails-works.md, line 45 at r6 (raw file):
If you have used the provided generator, these bundles will automatically be added to your `.gitignore` to prevent extraneous noise from re-generated code in your pull requests. You will want to do this manually if you do not use the provided generator. You can stop React on Rails from modifying or creating the `assets:precompile` task, by setting a `REACT_ON_RAILS_PRECOMPILE` environment variable to `no`, `false`, `n` or `f`.
Should this be set on the ENV and not in the config file?
I'm thinking of reasons for one or the other.
docs/rails/rails-engine-integration.md, line 19 at r6 (raw file):
+ In your `lib/tasks/<engine_name>_tasks.rake`: ```ruby Rake.application.remove_task('react_on_rails:assets:compile_environment')
since we're removing this, I think we should recommend setting the config value to false
In your ENV
REACT_ON_RAILS_PRECOMPILE=false
There was a problem hiding this comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @justin808)
docs/guides/how-react-on-rails-works.md, line 45 at r6 (raw file):
Previously, justin808 (Justin Gordon) wrote…
Should this be set on the ENV and not in the config file?
I'm thinking of reasons for one or the other.
In this case, I'd say that it doesn't make much of a difference except possibly for the ease of changing the value on CI.
docs/rails/rails-engine-integration.md, line 19 at r6 (raw file):
Previously, justin808 (Justin Gordon) wrote…
since we're removing this, I think we should recommend setting the config value to false
In your ENV
REACT_ON_RAILS_PRECOMPILE=false
Should that recommendation go in the configuration doc or in this doc?
lib/tasks/assets.rake, line 10 at r3 (raw file):
Previously, justin808 (Justin Gordon) wrote…
Hi @Judahmeek, we need documentation and a changelog entry for this one. I'll hold off on merging so we don't forget.
Done
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Judahmeek)
docs/guides/how-react-on-rails-works.md, line 45 at r6 (raw file):
Previously, Judahmeek (Judah Meek) wrote…
In this case, I'd say that it doesn't make much of a difference except possibly for the ease of changing the value on CI.
I think in most cases, people should not set this, so an ENV value is fine for the weird case.
docs/rails/rails-engine-integration.md, line 19 at r6 (raw file):
Previously, Judahmeek (Judah Meek) wrote…
Should that recommendation go in the configuration doc or in this doc?
I think this doc, and only if using a custom build configuration since we ignore the setting for the default rails/webpacker way.
There was a problem hiding this comment.
Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @justin808)
docs/rails/rails-engine-integration.md, line 19 at r6 (raw file):
Previously, justin808 (Justin Gordon) wrote…
I think this doc, and only if using a custom build configuration since we ignore the setting for the default rails/webpacker way.
Done
Fixes #1368.
This change is