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

Clean up, pretty up code for helper, move some code to inclusion file. #32

Closed
justin808 opened this issue Sep 20, 2015 · 6 comments
Closed

Comments

@justin808
Copy link
Member

The code right now in the react_on_rails gem looks horrible due to trying to preserve the correct white space. Here's the generated code:

And here's the files that generate it:

@samnang @alexfedoseev @mapreal19 @josiasds Any ideas on how best to refactor this?

BTW -- be super careful before accepting any commits that might mess with this formatting.

@Johnnycus Let's write a unit test that verifies that the generated JS stays the same, and that will enable us to do some refactoring.

@samnang
Copy link
Contributor

samnang commented Sep 21, 2015

@justin808 For indentation in heredoc, we could use http://api.rubyonrails.org/classes/String.html#method-i-strip_heredoc to remove white spaces from beginning.

@justin808
Copy link
Member Author

@samnang We originally did it that way, but it's super hard as well for changing the indentation. The code is also going to be slightly faster not using it. On a somewhat related note, we should maybe put some of the code for the client side into a file included in application.js.

Take a look at: https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/spec/sample_generated_js/client-generated.js

I think we only need a few parameters to a client side method call:

  1. name of globally exposed variable (property to be put on window): 'helloWorldES5Data0'
  2. name of id for element: 'HelloWorldES5-react-component-0'
  3. name of component: 'HelloWorldES5'
  4. trace: boolean
  5. expect turbolinks: boolean

@justin808 justin808 changed the title Clean up, pretty up code for helper Clean up, pretty up code for helper, move some code to inclusion file. Sep 21, 2015
@szyablitsky
Copy link
Contributor

I think that moving javascript into .js file which should be included in Rails' application.js is a good idea. If we need to add some runtime logic, than we can change extension to .js.erb and do some preprocessing for javascript code. And even cache it all or partially for performance gain.

@justin808
Copy link
Member Author

@szyablitsky Let me know if you'd like to do this. Or anybody else does, please let me know.

@justin808
Copy link
Member Author

@samnang and I made amazing progress from this today.

@justin808
Copy link
Member Author

Fixed by #44.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants