Skip to content

Replace nulls in props from Rails with undefined #1293

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

Merged
merged 11 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,4 @@ Naming/RescuedExceptionsVariableName:

Metrics/BlockLength:
Exclude:
- 'test/**/*_test.rb'
- 'test/**/*_test.rb'
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ Changes since last non-beta release.

_Please add entries here for your pull requests that are not yet released._

#### Added
- Added option to replace `null`s in props with `undefined` via `config.react.null_to_undefined_props` in `config/application.rb` #1293

## [3.0.0] - 2023-08-14

### Breaking Changes
Expand Down
26 changes: 24 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,16 @@ Read the [full review here](https://clutch.co/profile/shakacode#reviews?sort_by=
- [Upgrading](#upgrading)
- [2.7 to 3.0](#27-to-30)
- [2.3 to 2.4](#23-to-24)
- [Other features](#other-features)
- [Replace `null` with `undefined` in props](#replace-null-with-undefined-in-props)
- [Common Errors](#common-errors)
- [Getting warning for `Can't resolve 'react-dom/client'` in React < 18](#getting-warning-for-cant-resolve-react-domclient-in-react--18)
- [Undefined Set](#undefined-set)
- [Using TheRubyRacer](#using-therubyracer)
- [HMR](#hmr)
- [Related Projects](#related-projects)
- [Contributing](#contributing)
- [Supporters](#supporters)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->

Expand Down Expand Up @@ -528,7 +531,6 @@ use it like so:
ReactUJS.getConstructor = ReactUJS.constructorFromRequireContext(require.context('components', true));
```


## Server-Side Rendering

You can render React components inside your Rails server with `prerender: true`:
Expand Down Expand Up @@ -801,6 +803,26 @@ For the vast majority of cases this will get you most of the migration:
- add `import PropTypes from 'prop-types'` (Webpacker only)
- re-run `bundle exec rails webpacker:install:react` to update npm packages (Webpacker only)

## Other features

### Replace `null` with `undefined` in props

React-Rails converts `nil` to `null` while parsing props from Ruby to JavaScript. Optionally, you can configure React-Rails to parse `nil` values to `undefined` as per the following:

```ruby
# config/application.rb
module TheAppName
class Application < Rails::Application
# ...
# Set to true to convert null values in props into undefined
config.react.null_to_undefined_props = true
# ...
end
end
```

More information in: [discussion#1272](https://github.com/reactjs/react-rails/discussions/1272).

## Common Errors
### Getting warning for `Can't resolve 'react-dom/client'` in React < 18

Expand Down Expand Up @@ -857,7 +879,7 @@ By contributing to React-Rails, you agree to abide by the [code of conduct](http

You can always help by submitting patches or triaging issues. Even offering reproduction steps to issues is incredibly helpful!

# Supporters
## Supporters

The following companies support the development of this and other open-source projects maintained by ShakaCode by providing licenses to the ShakaCode team. ShakaCode stands by the usefulness of these products!

Expand Down
11 changes: 1 addition & 10 deletions gemfiles/base.gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ..
specs:
react-rails (2.7.1)
react-rails (3.0.0)
babel-transpiler (>= 0.7.0)
connection_pool
execjs
Expand Down Expand Up @@ -169,10 +169,6 @@ GEM
net-smtp (0.3.3)
net-protocol
nio4r (2.5.9)
nokogiri (1.14.3-x86_64-darwin)
racc (~> 1.4)
nokogiri (1.13.8-x86_64-linux)
racc (~> 1.4)
nokogiri (1.13.8-x86_64-linux)
racc (~> 1.4)
notiffany (0.1.3)
Expand Down Expand Up @@ -238,10 +234,6 @@ GEM
timeout (0.3.2)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
webdrivers (5.2.0)
nokogiri (~> 1.6)
rubyzip (>= 1.3.0)
selenium-webdriver (~> 4.0)
websocket (1.2.9)
websocket-driver (0.7.5)
websocket-extensions (>= 0.1.0)
Expand Down Expand Up @@ -271,7 +263,6 @@ DEPENDENCIES
react-rails!
selenium-webdriver
test-unit (~> 2.5)
webdrivers

BUNDLED WITH
2.4.9
7 changes: 1 addition & 6 deletions gemfiles/shakapacker.gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ..
specs:
react-rails (2.7.1)
react-rails (3.0.0)
babel-transpiler (>= 0.7.0)
connection_pool
execjs
Expand Down Expand Up @@ -244,10 +244,6 @@ GEM
timeout (0.3.2)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
webdrivers (5.2.0)
nokogiri (~> 1.6)
rubyzip (>= 1.3.0)
selenium-webdriver (~> 4.0)
websocket (1.2.9)
websocket-driver (0.7.5)
websocket-extensions (>= 0.1.0)
Expand Down Expand Up @@ -278,7 +274,6 @@ DEPENDENCIES
selenium-webdriver
shakapacker (= 7.0.2)
test-unit (~> 2.5)
webdrivers

BUNDLED WITH
2.4.9
7 changes: 1 addition & 6 deletions gemfiles/sprockets_3.gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ..
specs:
react-rails (2.7.1)
react-rails (3.0.0)
babel-transpiler (>= 0.7.0)
connection_pool
execjs
Expand Down Expand Up @@ -246,10 +246,6 @@ GEM
turbolinks-source (5.2.0)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
webdrivers (4.2.0)
nokogiri (~> 1.6)
rubyzip (>= 1.3.0)
selenium-webdriver (>= 3.0, < 4.0)
websocket-driver (0.7.5)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.5)
Expand Down Expand Up @@ -282,7 +278,6 @@ DEPENDENCIES
sprockets-rails
test-unit (~> 2.5)
turbolinks (~> 5)
webdrivers

BUNDLED WITH
2.4.9
7 changes: 1 addition & 6 deletions gemfiles/sprockets_4.gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ..
specs:
react-rails (2.7.1)
react-rails (3.0.0)
babel-transpiler (>= 0.7.0)
connection_pool
execjs
Expand Down Expand Up @@ -246,10 +246,6 @@ GEM
turbolinks-source (5.2.0)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
webdrivers (4.2.0)
nokogiri (~> 1.6)
rubyzip (>= 1.3.0)
selenium-webdriver (>= 3.0, < 4.0)
websocket-driver (0.7.5)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.5)
Expand Down Expand Up @@ -282,7 +278,6 @@ DEPENDENCIES
sprockets-rails
test-unit (~> 2.5)
turbolinks (~> 5)
webdrivers

BUNDLED WITH
2.4.9
16 changes: 15 additions & 1 deletion lib/react/rails/component_mount.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ def generate_html_options(name, options, props, prerender_options)
unless prerender_options == :static
html_options[:data].tap do |data|
data[:react_class] = name
data[:react_props] = (props.is_a?(String) ? props : props.to_json)
data[:react_props] = props_to_json(
props,
null_to_undefined: Dummy::Application.config.react.null_to_undefined_props
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justin808 should we use Dummy::Application here? Dummy is just for tests. I suspect it should be Rails.configuration / Rails.application.config (both are the same), no?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Agree.

@ahangarha can you get a PR in for this ASAP?

Or @lcmen if you can get us a PR, I'll merge ASAP.

Extra credit if you provide a test that shows this bug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the PR was not ready for merge. To me it was still a WIP with good progress.

I am on it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey folks,

Just wanted to chime in that this exact issue is breaking the 3.1.0 gem for me on a fresh Rails app.

Following the installation steps outlined in the README produces:

> rails s
=> Booting Puma
=> Rails 7.0.7 application starting in development
=> Run `bin/rails server --help` for more startup options
Puma starting in single mode...
* Puma version: 5.6.7 (ruby 3.1.2-p20) ("Birdie's Version")
*  Min threads: 5
*  Max threads: 5
*  Environment: development
*          PID: 19068
* Listening on http://127.0.0.1:3000
* Listening on http://[::1]:3000
Use Ctrl-C to stop
Started GET "/" for 127.0.0.1 at 2023-08-20 20:15:25 -0400
Processing by HomeController#index as HTML
  Rendering layout layouts/application.html.erb
  Rendering home/index.html.erb within layouts/application
  Rendered home/index.html.erb within layouts/application (Duration: 2.8ms | Allocations: 4135)
  Rendered layout layouts/application.html.erb (Duration: 2.9ms | Allocations: 4247)
Completed 500 Internal Server Error in 8ms (ActiveRecord: 0.0ms | Allocations: 8205)



ActionView::Template::Error (uninitialized constant React::Rails::ComponentMount::Dummy

              null_to_undefined: Dummy::Application.config.react.null_to_undefined_props
                                 ^^^^^):
    2: <p>Find me in app/views/home/index.html.erb</p>
    3:
    4: <!-- erb: paste this in view -->
    5: <%= react_component("HelloWorld", { greeting: "Hello from react-rails." }) %>

app/views/home/index.html.erb:5
^C- Gracefully stopping, waiting for requests to finish
=== puma shutdown: 2023-08-20 20:21:16 -0400 ===
- Goodbye!
Exiting

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please upgrade to 3.1.1.

)
data[:hydrate] = "t" if prerender_options

num_components = @cache_ids.count { |c| c.start_with? name }
Expand All @@ -67,6 +70,17 @@ def generate_html_options(name, options, props, prerender_options)
html_options
end

def props_to_json(props, options = { null_to_undefined: false })
return props if props.is_a?(String)
return props.to_json unless options[:null_to_undefined]

# This regex matches key:value with null values while ensuing no string with similar
# pattern gets matched. It doesn't include null values in arrays.
props.to_json
.gsub(/([^\\]":)null([,}\]])/, '\1undefined\2') # match simple null values
.gsub(/([^\\]":(\[[^\\"]+,|\[))null([,\]])/, '\1undefined\3') # Match nulls in array
end

def rendered_tag(html_options, &block)
html_tag = html_options[:tag] || :div

Expand Down
1 change: 1 addition & 0 deletions lib/react/rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Railtie < ::Rails::Railtie
config.react.jsx_transformer_class = nil # defaults to BabelTransformer
config.react.camelize_props = false # pass in an underscored hash but get a camelized hash
config.react.sprockets_strategy = nil # how to attach JSX to the asset pipeline (or `false` for none)
config.react.null_to_undefined_props = false # Set to true to convert null values in props into undefined

# Server rendering:
config.react.server_renderer_pool_size = 1 # increase if you're on JRuby
Expand Down
88 changes: 88 additions & 0 deletions test/react/rails/component_mount_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "test_helper"

# rubocop:disable Metrics/ClassLength
class ComponentMountTest < ActionDispatch::IntegrationTest
module DummyRenderer
def self.render(component_name, props, _prerender_options)
Expand Down Expand Up @@ -128,5 +129,92 @@ def self.react_rails_prerenderer

assert_equal %(<div>rendered Foo with {&quot;ok&quot;:true}</div>), rendered_component
end

test "#react_component sets null props to undefined when null_to_undefined_props set to true" do
app.config.react.null_to_undefined_props = true

@helper.setup(DummyController)
rendered_component = @helper.react_component("Foo", { bar: nil, content: 'bar":null,' })

assert_includes rendered_component, '&quot;bar&quot;:undefined,&quot;content&quot;:&quot;bar\\&quot;:null,&quot;'
end

test "#react_component passes null props as null when null_to_undefined_props set to false" do
app.config.react.null_to_undefined_props = false

@helper.setup(DummyController)
rendered_component = @helper.react_component("Foo", { bar: nil, content: 'bar":null,' })

assert_includes rendered_component, "&quot;bar&quot;:null,&quot;content&quot;:&quot;bar\\&quot;:null,&quot;"
end

test "#props_to_json doesn't converts null values to undefined be default" do
props = { name: nil }
expected_json = '{"name":null}'
component_mount = React::Rails::ComponentMount.new

actual_json = component_mount.send(:props_to_json, props)

assert_equal(expected_json, actual_json)
end

test "#props_to_json converts null values to undefined with null_to_undefined: true option" do
props = { bar: nil, content: 'bar":null,' }
expected_json = '{"bar":undefined,"content":"bar\\":null,"}'
component_mount = React::Rails::ComponentMount.new

actual_json = component_mount.send(:props_to_json, props, { null_to_undefined: true })

assert_equal(expected_json, actual_json)
end

test "#props_to_json converts null values in arrays to undefined with null_to_undefined: true option" do
props = { items1: [nil], items2: [1, nil], items3: [nil, 1], items4: [1, nil, 2] }
expected_json = '{"items1":[undefined],"items2":[1,undefined],"items3":[undefined,1],"items4":[1,undefined,2]}'
component_mount = React::Rails::ComponentMount.new

actual_json = component_mount.send(:props_to_json, props, { null_to_undefined: true })

assert_equal(expected_json, actual_json)
end

test "#props_to_json doesnt converts null-like values in arrays to undefined with null_to_undefined: true option" do
props = {
items1: "[null]",
items2: "[1,null]",
items3: "[null,1]",
items4: "[1,null,2]",
items5: '["a",null]',
items6: '[null,"b"]',
items7: '["a",null,"b"]',
items8: '["a",nullx,"b"]'
}
expected_json = '{"items1":"[null]","items2":"[1,null]","items3":"[null,1]","items4":"[1,null,2]",' \
'"items5":"[\"a\",null]","items6":"[null,\"b\"]","items7":"[\"a\",null,\"b\"]"' \
',"items8":"[\"a\",nullx,\"b\"]"}'
component_mount = React::Rails::ComponentMount.new

actual_json = component_mount.send(:props_to_json, props, { null_to_undefined: true })

assert_equal(expected_json, actual_json)
end

test "#props_to_json doesnt converts null values in nested arrays to undefined with null_to_undefined: true" do
props = {
items1: nil,
items2: [1, nil, 2],
items3: nil,
items4: "[1, null, 2]",
items5: nil
}
expected_json = '{"items1":undefined,"items2":[1,undefined,2],"items3":undefined,"items4":"[1, null, 2]"' \
',"items5":undefined}'
component_mount = React::Rails::ComponentMount.new

actual_json = component_mount.send(:props_to_json, props, { null_to_undefined: true })

assert_equal(expected_json, actual_json)
end
end
end
# rubocop:enable Metrics/ClassLength