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

Allow default content in content_tag in case of noscript/render failure #59

Open
jtokoph opened this Issue Oct 2, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@jtokoph

jtokoph commented Oct 2, 2018

I would like to specify some default content that renders serverside within the content tag generated by the react_component helper. I've noticed that it defaults to calling content_tag with a nil body and created my own react_component_with_defaults helper like so:

def react_component_with_defaults(name, props = {}, options = {}, &block)
    tag = options.delete(:tag) || :div
    data = { data: { 'react-class' => name, 'react-props' => props.to_json } }

    content = block_given? ? capture(&block) : nil
    content_tag(tag, content, options.deep_merge(data))
end

This would theoretically allow me to pass a block with default content to render serverside as a fallback in the event of someone having js disabled or a mounting issue when the react js finally executes.

<%= react_component_with_defaults 'SomeComponent', { some_prop: 1 } do %>
    <div>default content here</div>
<% end %>

Unfortunately the javascript side of webpacker-react prevents rendering if there is any innerHTML to the content tag (See relevant line)

Are there any arguments against adding this as a feature? Or is this just something that react wouldn't like with automatic hydration attempts? I'm not knowledgeable enough on react internals to know if this is a bad idea or not.

@BookOfGreg

This comment has been minimized.

Show comment
Hide comment
@BookOfGreg

BookOfGreg Oct 4, 2018

This comes up relatively often reactjs/react-rails#901 (comment)
reactjs/react-rails#913 (comment)

In general Ruby and JS contexts can't really share anything other than props. You could pass in child html though and dangerously render it to simulate react wrapping ruby content but not really recommended due to the dangerously set inner html call.

BookOfGreg commented Oct 4, 2018

This comes up relatively often reactjs/react-rails#901 (comment)
reactjs/react-rails#913 (comment)

In general Ruby and JS contexts can't really share anything other than props. You could pass in child html though and dangerously render it to simulate react wrapping ruby content but not really recommended due to the dangerously set inner html call.

@jtokoph

This comment has been minimized.

Show comment
Hide comment
@jtokoph

jtokoph Oct 6, 2018

I don't think I was clear enough. I don't care about the content in the block surviving. I just want some default server rendered content that will display if the user has javascript disabled or in the event that the initial mount fails. Think of it like having a <noscript>..some serverside rendered version of the app...</noscript>. My current solution is to use the noscript tag, but it doesn't kick in if there was some other javascript error before mounting. If react mounts properly, it can destroy the current markup in the tag and render it's own tree. I'm fine with that, if that's how react would behave (I wasn't totally sure).

jtokoph commented Oct 6, 2018

I don't think I was clear enough. I don't care about the content in the block surviving. I just want some default server rendered content that will display if the user has javascript disabled or in the event that the initial mount fails. Think of it like having a <noscript>..some serverside rendered version of the app...</noscript>. My current solution is to use the noscript tag, but it doesn't kick in if there was some other javascript error before mounting. If react mounts properly, it can destroy the current markup in the tag and render it's own tree. I'm fine with that, if that's how react would behave (I wasn't totally sure).

@BookOfGreg

This comment has been minimized.

Show comment
Hide comment
@BookOfGreg

BookOfGreg Oct 7, 2018

Ah yes it seems like I misunderstood what you were after, sorry for that.
You wonder if there's a nice way of doing it within the react component taking advantage of the lifecycle hooks. Such as have a default noscript render, then remove it on something like componentWillMount which only triggers when JS is mounting client-side.

BookOfGreg commented Oct 7, 2018

Ah yes it seems like I misunderstood what you were after, sorry for that.
You wonder if there's a nice way of doing it within the react component taking advantage of the lifecycle hooks. Such as have a default noscript render, then remove it on something like componentWillMount which only triggers when JS is mounting client-side.

@renchap

This comment has been minimized.

Show comment
Hide comment
@renchap

renchap Oct 7, 2018

Owner

I am open to have this feature implemented. With an optional block argument on the react_component helper we should not need to add a new helper (like your react_component_with_defaults).
One drawback is that if your pack file is loaded with async there might be a flash of content on page load, as the browser will display the HTML content, and our JS will remove the content from the DOM before calling ReactDOM.render.
A solutions would be to add a <noscript> tag around the block provided to react_component, so it will only be rendered if JS is disabled. Would this work for you?

Owner

renchap commented Oct 7, 2018

I am open to have this feature implemented. With an optional block argument on the react_component helper we should not need to add a new helper (like your react_component_with_defaults).
One drawback is that if your pack file is loaded with async there might be a flash of content on page load, as the browser will display the HTML content, and our JS will remove the content from the DOM before calling ReactDOM.render.
A solutions would be to add a <noscript> tag around the block provided to react_component, so it will only be rendered if JS is disabled. Would this work for you?

@jtokoph

This comment has been minimized.

Show comment
Hide comment
@jtokoph

jtokoph Oct 8, 2018

 I'm already using the <noscript> tag in conjunction with a react_component call for progressive enhancement purposes:

<%= react_component 'CommentFeedWithFancyInteractivity', comments: @comments %>
<noscript>
  <% @comments.each do |comment| %>
    <div>{comment.author}: {comment.body}</div>
  <% end %>
</noscript>

This pretty much gets the job done. I think the only improvement would be in the super rare case that javascript is enabled, but somehow the mounting of the component failed. Maybe an error in the bootstrapping of the react component or app, or some browser incompatibility. It's in this case that the react version of the content never shows because of the error, and the noscript version of the content doesn't display because the browser has javascript enabled. I think supporting serverside rendering of the react components would also solve this unique case.

I wouldn't say that this is a defect or shortcoming of this gem at all. I only opened the issue because my brain thought "Wouldn't it be cool if it worked like this?", so I tried it.

I wouldn't put this high on any priority list since <noscript> gets 90% of the way. It could even be closed if you think it's too niche.

jtokoph commented Oct 8, 2018

 I'm already using the <noscript> tag in conjunction with a react_component call for progressive enhancement purposes:

<%= react_component 'CommentFeedWithFancyInteractivity', comments: @comments %>
<noscript>
  <% @comments.each do |comment| %>
    <div>{comment.author}: {comment.body}</div>
  <% end %>
</noscript>

This pretty much gets the job done. I think the only improvement would be in the super rare case that javascript is enabled, but somehow the mounting of the component failed. Maybe an error in the bootstrapping of the react component or app, or some browser incompatibility. It's in this case that the react version of the content never shows because of the error, and the noscript version of the content doesn't display because the browser has javascript enabled. I think supporting serverside rendering of the react components would also solve this unique case.

I wouldn't say that this is a defect or shortcoming of this gem at all. I only opened the issue because my brain thought "Wouldn't it be cool if it worked like this?", so I tried it.

I wouldn't put this high on any priority list since <noscript> gets 90% of the way. It could even be closed if you think it's too niche.

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