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

Putting back in pretty printing of JSON props? #799

Closed
justin808 opened this issue Apr 10, 2017 · 7 comments
Closed

Putting back in pretty printing of JSON props? #799

justin808 opened this issue Apr 10, 2017 · 7 comments

Comments

@justin808
Copy link
Member

Any votes for putting pretty printing of the JSON props in the HTML page source? We were doing that at first, but this resulted in some bugs.

@cheremukhin23 is working on #787 which would put that back in, along with some tests.

I’m not 100% sure we even want this, as it’s probably just as well to put a debugger statement or console log and see the props that way.

I’d really like some community feedback on this one.

Maybe if nobody here cares, it’s best to leave this one out?

@nratter
Copy link
Contributor

nratter commented Apr 10, 2017

I think it's unnecessary as you can use something like the following...

<pre>
  {JSON.stringify(this.props.data.toJS(), null, 2)}
</pre>

@squadette
Copy link
Contributor

Pretty-printing was second reason behind me driving this change forward.

Of course you can do pre-thing, console thing, or just mentally interpret JSON from attributes, but showing pretty-printed JSON in your source makes it so much more convenient and useful. You can do Ctrl-F in your view source easily, even if you don't have the same level of advanced developer console as is in Chrome (there are other browsers btw).

This is one of the small changes which increase developer happiness, which Ruby and Rails are famous for.

I do care. As I said, to fix the ActiveRecord issue we could probably add as_json call.

Thank you,

@justin808
Copy link
Member Author

justin808 commented Apr 13, 2017

@squadette can you help us with the change? and can we make it configurable in config/initializers/react_on_rails.rb.

See this comment: #787 (comment)

I'm OK with this if it's WELL TESTED given the cases where this broke.

@justin808
Copy link
Member Author

@squadette: Now that I merged #812, it would be very easy to do the pretty printing. Maybe this is the last thing before we go to beta for 7.0?

@justin808
Copy link
Member Author

@squadette any interest in taking a crack at this one?

@ebauger
Copy link

ebauger commented Aug 6, 2018

@justin808 I'll check this week what I can do

@justin808
Copy link
Member Author

@ebauger Let me know if you'd like an invite to our Slack. Email me.

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