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

[Rails 3.2][v6.9.0] Parsing error when fetching ReactOnRails context from script tag #782

Closed
Ynote opened this issue Mar 30, 2017 · 27 comments

Comments

@Ynote
Copy link
Contributor

Ynote commented Mar 30, 2017

Hi,

I updated the gem to the version 6.9.0 so it can be compatible with the node module (6.9.0).
I'm using it with Rails 3.2.

Unexpectedly, I got this parsing error:

Uncaught SyntaxError: Unexpected token i in JSON at position 18
    at JSON.parse (<anonymous>)
    at parseRailsContext (app.js:23226)
    at reactOnRailsPageLoaded (app.js:23235)
    at HTMLDocument.<anonymous> (app.js:23289)

This is the output I have in the script tag generated by the helper prepend_render_rails_context:

<script id="js-react-on-rails-context" type="application/json">
{
  inMailer: false,
  i18nLocale: fr,
  i18nDefaultLocale: fr,
  href: http://localhost:3000/,
  location: /,
  scheme: http,
  host: localhost,
  port: 3000,
  pathname: /,
  search: null,
  httpAcceptLanguage: fr,en;q=0.8,en-US;q=0.6,zh-CN;q=0.4,zh;q=0.2,tr;q=0.2,nl;q=0.2,it;q=0.2,de;q=0.2,es;q=0.2,
  serverSide: false
}
</script>

The generated output doesn't seem to be formatted as json so it cannot be parsed correctly by the javascript. Is there something that I should have done in my configuration?

Thank you very much for your help (and your great great project)!

@justin808
Copy link
Member

@cheremukhin23 It seems the changes don't work for Rails 3.x.

@Ynote Any chance that you can try to submit a PR to fix this?

@Ynote can you downgrade to 6.8.x. I suspect that Rails 3.2 is not compatible with the latest changes.

  1. This call creates the content tag for the context
  2. Here is the call that formats the content

https://github.com/cheremukhin23/react_on_rails/blob/82adca11e08651b91f384b1ca23ca498eda9e3d2/app/helpers/react_on_rails_helper.rb#L225

  def json_safe_and_pretty(something)
    if Rails.env.development?
      ERB::Util.json_escape(JSON.pretty_generate(something.as_json))
    else
      ERB::Util.json_escape(something.to_json)
    end
  end

I'm puzzled by how this code left out the quotes for the strings.

@justin808 justin808 changed the title [v6.9.0] Parsing error when fetching ReactOnRails context from script tag [v6.9.0] [RAILS 3.2?] Parsing error when fetching ReactOnRails context from script tag Mar 31, 2017
@justin808
Copy link
Member

@Ynote Please see if the fix in 6.9.1 helps you.

@kulakowka
Copy link

I have this problem with rails 5.0.2.
react-on-rails 6.9.1

@justin808 justin808 changed the title [v6.9.0] [RAILS 3.2?] Parsing error when fetching ReactOnRails context from script tag [v6.9.0] Parsing error when fetching ReactOnRails context from script tag Mar 31, 2017
@justin808
Copy link
Member

@kulakowka @Ynote We're not seeing this on the https://github.com/shakacode/react-webpack-rails-tutorial nor with simple generated apps, all with Rails 5.

Can you please create a reproduction scenario so we can fix this?

@kulakowka
Copy link

I performed the following actions:

  1. remove npm_modules
  2. npm install

@justin808
Copy link
Member

This is from shakacode/react-webpack-rails-tutorial#370
2017-03-30_23-33-45

@kulakowka
Copy link

My source file:
2017-03-31 12 45 53

@kulakowka
Copy link

Apparently I forgot to update the gem.

Here's how I fix this:

  • I deleted the node_modules folder. rm -rf client/node_modules
  • Updated gem react_on_rails to 6.9.1 bundle update react_on_rails
  • Updated the npm package react-on-rails to 6.9.1 npm install react-on-rails @ latest --save
  • Run the command: npm install

@Ynote
Copy link
Contributor Author

Ynote commented Mar 31, 2017

@justin808 sorry the late reply. I try to update to the version 6.9.1 with the fix. It didn't work. The quotes are still not there.

I have a big (and old) Rails application. I don't really know how to create a reproductible scenario. I'll try with a plain new app (on Rails 3.2) and check if I still have the bug.

For the moment, I just downgraded to 6.8. I'm going to dig deeper in my app and the interaction with ReactOnRails and keep you up to date.

Thank you very much for you help.

@kulakowka
Copy link

The problem is that npm itself has updated the package version.

My version of the package.json was:

"react-on-rails": "^6.0.2",

And Gemfile.lock

react_on_rails (6.5.1)

When I ran npm install command, the package was updated to 6.9.1
In this case, the version of the gem in Gemfile.lock remains the same (react_on_rails 6.5.1)

I think you should do a check or at least mention this in the documentation.

@Ynote
Copy link
Contributor Author

Ynote commented Mar 31, 2017

@justin808 The output issue seems to be related to the json_escape method from ERB::Util. In Rails 3.x, the method does not return valid json (cf. rails/rails@a38e653a6d512de3d221).

We are trying to update the Rails version of our application but it is a huge task and it won't be done immediately. Do you have a suggestion for me to deal with that?

@justin808
Copy link
Member

@Ynote: I think we need to copy the code from the current version of that method and put in RoR and use that code for Rails version less than the version that is current.

@kulakowka: When your rails app starts, it should be complaining if the versions don't match. Are you not seeing errors in the logs? @robwise Maybe we should throw an exception?

CC: @cheremukhin23

@kulakowka
Copy link

@justin808 What does this error look like?

@justin808
Copy link
Member

https://github.com/shakacode/react_on_rails/blob/master/lib/react_on_rails/version_checker.rb#L29

Should be a logger.warn:

      msg = "**WARNING** ReactOnRails: ReactOnRails gem and node package MAJOR versions do not match\n" \
            "                     gem: #{gem_version}\n" \
            "            node package: #{node_package_version.raw}\n" \
            "Ensure the installed MAJOR version of the gem is the same as the MAJOR version of \n"\
            "your installed node package."

@kulakowka
Copy link

@justin808 I'll check it out on Monday.

@justin808
Copy link
Member

@robwise, @kulakowka: Maybe we should do a logger.error?

@justin808 justin808 changed the title [v6.9.0] Parsing error when fetching ReactOnRails context from script tag [Rails 3.2][v6.9.0] Parsing error when fetching ReactOnRails context from script tag Apr 1, 2017
@kulakowka
Copy link

@justin808 yes. It would be very convenient.

@justin808
Copy link
Member

justin808 commented Apr 2, 2017

We have a fix for @Ynote in #787.

@Ynote do you want to try this branch out and see if this fixes your issue? You will need to use github references in both your Gemfile and your client/package.json.

@Ynote
Copy link
Contributor Author

Ynote commented Apr 3, 2017

Hi,

I tried this branch and the fix works. But, I have to detail a little bit more what I have done because I ran into another error.

When fetching the fix, I got another error when the helper parsed the props key in the method render_redux_store_data:
NoMethodError: undefined method key?' for #<JSON::Ext::Generator::State:0x007ffd9a381270>.

In my props, I have something like this:

{
  entities: {
    projects: {
      byId: {
        1282: project.find(1282),
        1509: project.find(1509)
      },
      selected: [1509]
    }
  }
}

Here, the JSON.pretty_generate doesn't work correctly on the ActiveRecord model. It seems that this is related to the gem multi_json (cf. intridea/multi_json#86).

I have so many other gems in my old RoR application that updating multi_json wasn't possible. So, my workaround now is to pass a JSON object, instead of an ActiveRecord model:

{
  entities: {
    projects: {
      byId: {
        1282: project.find(1282).to_json,
        1509: project.find(1509).to_json
      },
      selected: [1509]
    }
  }
}

It seems to work correctly. I don't know if what I've done is correct or if I could have done better.

@justin808
Copy link
Member

JSON.pretty_generate has been removed for now. @Ynote Please try 6.9.3.

@mikemerritt
Copy link

I ran in to this issue as well and 6.9.3 seems to have fixed it.

@Ynote
Copy link
Contributor Author

Ynote commented Apr 11, 2017

Hi @justin808,

I am really sorry for the late late reply. #791 is not enough in my stack to handle the issue.

It doesn't work for me with 6.9.3 because I still need #787 to handle the simple quote removing in Rails 3.x.

@justin808
Copy link
Member

@Ynote can you try using #787 as a branch locally to confirm that this works for you. I will then merge after I get your confirmation.

@justin808 justin808 reopened this Apr 11, 2017
@Ynote
Copy link
Contributor Author

Ynote commented Apr 12, 2017

@justin808 I just tested #787. It works perfectly with my stack.

Thank you so much for your help and your investigation!

@justin808
Copy link
Member

@Ynote Any chance you might be able to help us with a few tests for #787? so we can get that merged?

@Ynote
Copy link
Contributor Author

Ynote commented Apr 19, 2017

Hi @justin808,

I just pushed #812. I'm sorry, I didn't use Reviewable, I'm not used to it. I hope the PR will be ok.

@Ynote
Copy link
Contributor Author

Ynote commented Apr 23, 2017

Just for the record on this issue:

The eating quotes issue and the special characters \u2028 and \u2029 escaping issue on ERB::Util.json_escape helper method were resolved in Rails 4.1 (rails/rails#13073).

More information about the fix: https://github.com/shakacode/react_on_rails/pull/812/files#diff-1958662b8ed5b2a812bdc2f427f01bc2R15.

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

4 participants