Skip to content

Keep variable name case sensitive on assert_react_component props. #979

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

Conversation

huacnlee
Copy link
Contributor

@huacnlee huacnlee commented Apr 1, 2019

We need make sure the prop name is correct, because JavaScript used that it's case sensitive.

for example:

<%= react_component "InlineEditor", name: "comment[body_sml]", directUploadURL: rails_direct_uploads_url, blobURLTemplate: "/uploads/:id" %>
const InlineEditor extends Component {
  render() {
    const { directUploadURL, blobURLTemplate } = this.props;
  }
}

@BookOfGreg

@BookOfGreg BookOfGreg added the bug label Apr 1, 2019
@@ -4,7 +4,7 @@
</ul>

<div id='component-parent'>
<%= react_component 'GreetingMessage', { name: @name, last_name: "Last #{@name}", info: { name: @name } }, { id: 'component', class: "greeting-message", prerender: @prerender } %>
<%= react_component 'GreetingMessage', { name: @name, lastName: "Last #{@name}", info: { name: @name, lastName: "Last #{@name}" } }, { id: 'component', class: "greeting-message", prerender: @prerender } %>
Copy link
Member

@BookOfGreg BookOfGreg Apr 1, 2019

Choose a reason for hiding this comment

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

Can you do the same in the other dummy applications?
That should make the tests pass.

test/dummy_sprockets/app/views/pages/show.html.erb
test/dummy_webpacker1/app/views/pages/show.html.erb
test/dummy_webpacker2/app/views/pages/show.html.erb

Copy link
Member

@BookOfGreg BookOfGreg Apr 1, 2019

Choose a reason for hiding this comment

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

@huacnlee huacnlee force-pushed the fix/assert_react_component_do_not_underscore branch from cbdfb42 to e4eb626 Compare April 1, 2019 10:33
@huacnlee
Copy link
Contributor Author

huacnlee commented Apr 1, 2019

Just updated.

BTW, why I run test on my local always have this errors:

$ rake test
bin/rails test /Users/jason/work/react-rails/test/server_rendered_html_test.rb:138

E

Error:
ServerRenderedHtmlTest#test_react_inline_component_rendering_(pre-rendered):
ActionView::Template::Error: couldn't find file 'turbolinks' with type 'application/javascript'
Checked in these paths: 
  /Users/jason/work/react-rails/test/dummy_sprockets/app/assets/config
  /Users/jason/work/react-rails/test/dummy_sprockets/app/assets/images
  /Users/jason/work/react-rails/test/dummy_sprockets/app/assets/javascripts
  /Users/jason/work/react-rails/test/dummy_sprockets/app/assets/stylesheets
  /Users/jason/work/react-rails/test/dummy_sprockets/vendor/assets/javascripts
  /Users/jason/work/react-rails/test/dummy_sprockets/vendor/assets/react

@BookOfGreg
Copy link
Member

Not sure why it happens but I saw that myself. I had to CD into each test directory and remove the node_modules folder from each one. The rake task will try npm install in each directory according to the rakefile.

We need make sure the prop name is correct, because JavaScript used that it's case sensitive.
@huacnlee huacnlee force-pushed the fix/assert_react_component_do_not_underscore branch from e4eb626 to 4158231 Compare April 1, 2019 13:51
@BookOfGreg BookOfGreg merged commit e8b2e4e into reactjs:master Apr 23, 2019
@BookOfGreg
Copy link
Member

Thanks for this one! I'll set an alarm so I can get it shipped out this week.

@huacnlee huacnlee deleted the fix/assert_react_component_do_not_underscore branch April 23, 2019 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants