Fix ReactDomClient bug #1278
Fix ReactDomClient bug #1278
Conversation
Judahmeek
force-pushed
the
fix-bug-with-requiring-react-dom-with-sporockets
branch
from
6233b2b to
b6811a0
Compare
| - run: bundle exec rake react:update | ||
| - run: bundle exec rake ujs:update | ||
| - run: ./check_for_uncommitted_files.sh |
There was a problem hiding this comment.
I think it is better to extract this logic into another rake task and use it in both:
- Release script
- CI
Is will be DRY and also prevents adding bash script in the root of the project.
| - 2.6.5 | ||
| # TODO restore testing against JRuby w/ therubyrhino | ||
| # - jruby-9.3.7.0 | ||
| ruby: [2.7] |
There was a problem hiding this comment.
Not related to this PR, but I think we should add ruby 3 to the matrix for our tests.
| @@ -39,7 +39,7 @@ task :create_release, %i[gem_version dry_run] do |_t, args| | |||
| puts 'Updating ujs:update' | |||
| Rake::Task['ujs:update'].invoke | |||
|
|
|||
| Release.commit_the_changes('Update pre-bundled react and React ujs') unless is_dry_run | |||
| Release.make_sure_react_and_ujs_are_updated_and_commited | |||
There was a problem hiding this comment.
With this change, we don't need the commit_the_changes method anymore.
| def make_sure_react_and_ujs_are_updated_and_commited | ||
| status = `git status --porcelain` | ||
|
|
||
| return if $CHILD_STATUS.success? && status == '' | ||
|
|
||
| error = if $CHILD_STATUS.success? | ||
| 'Running react:update and ujs:update resulted in uncommitted changes. Please commit those changes & confirm that CI passes before continuing.' | ||
| else | ||
| 'You do not have Git installed. Please install Git, and commit your changes before continuing' | ||
| end | ||
| raise(error) | ||
| end | ||
|
|
There was a problem hiding this comment.
As mentioned before, this can become a separate rake task
| - 2.6.5 | ||
| # TODO restore testing against JRuby w/ therubyrhino | ||
| # - jruby-9.3.7.0 | ||
| ruby: [2.7] |
| - run: bundle exec rake react:update | ||
| - run: bundle exec rake ujs:update | ||
| - run: ./check_for_uncommitted_files.sh |
react_ujs/src/reactDomClient.js
Outdated
| } | ||
| } | ||
|
|
||
| export default reactDomClient | ||
| export default reactDomClient |
There was a problem hiding this comment.
@Judahmeek set your edit to add the extra line at the end.
|
@Judahmeek we're getting a CI failure: https://github.com/reactjs/react-rails/actions/runs/5020783720/jobs/9002562807?pr=1278 |
Summary
This closes #1276 & #1277
Other Information
Remove this paragraph and mention any other important and relevant information such as benchmarks.
Pull Request checklist