Skip to content
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

CoffeeScript React component generator is fixed #768

Closed
wants to merge 4 commits into from

Conversation

ViXP
Copy link
Contributor

@ViXP ViXP commented Aug 11, 2017

CoffeeScript React component generator is fixed to modules export of component and to require the React object.
rails g react:component Sample --coffee
Before:

class @Sample extends React.Component
  render: ->
    `<div />`

After:

React = require('react')

class @Sample extends React.Component
  render: ->
    `<div />`

module.exports = @Sample

Copy link
Member

@BookOfGreg BookOfGreg left a comment

Choose a reason for hiding this comment

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

Sorry this one has taken so long to get to. Can you merge master in so I can see if tests now pass on this and we will look at getting this merged? I'm not seeing any failing tests on master for the moment so we may look to getting a test added that exercises the failing behaviour this fixes.

if options[:coffee]
%|React = require('react')\n|
else
%|var React = require("react")\n|
Copy link
Member

Choose a reason for hiding this comment

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

Would these work as import rather than requires if it's webpack?

if options[:coffee]
%|module.exports = @#{component_name}|
else
%|module.exports = #{component_name}|
Copy link
Member

Choose a reason for hiding this comment

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

Similar to above, could these be export default rather than module.exports?

@ViXP
Copy link
Contributor Author

ViXP commented Oct 1, 2017

Yeah, I've corrected the code you mentioned, it is right to stick to one of the possible syntaxes. Also, PropTypes was removed from base 'react' source code, so now it must be imported in such way: import PropTypes from 'prop-types'. I've added this line to the generator too.

@BookOfGreg
Copy link
Member

I've merged your code and commits into #798 and corrected the associated tests. Thank you for your help with Coffeescript! Could you test out that branch and see if it meets your needs?

@BookOfGreg BookOfGreg closed this Oct 2, 2017
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.

2 participants