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

Server side rendering react component fails #352

Closed
ediblecode opened this issue Aug 9, 2019 · 7 comments · Fixed by #415
Closed

Server side rendering react component fails #352

ediblecode opened this issue Aug 9, 2019 · 7 comments · Fixed by #415
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)

Comments

@ediblecode
Copy link

Server rendering of the react component fails in two places, because it's looking for either window or document (I'll point out where in the code, below).

This would happen with either a universal (isomorphic?) app or statically pre-rendering react components into HTML, for example with Gatsby.

Repro steps

  1. Create a Gatsby application.
  2. import AccessibleAutocomplete from "accessible-autocomplete/react";
  3. Render <AccessibleAutocomplete ...

Expected result

The render works OK, rendering static markup on the server that is rehydrated on the client

Actual result

The following errors occur:

  1. window is not defined because of a webpack issue
    2 document is not defined because of the hasPointerEvents function

Reason for window being undefined

This is because of issues with webpack 4 in the way that the UMD bundle is generated:
webpack/webpack#6642
webpack/webpack#6677
The fix can be as simple as: ankurk91/vue-flatpickr-component@c6f37ba

Reason for document being undefined

The hasPointerEvents function is called when the module loads, and looks for document.createElement
See https://github.com/alphagov/accessible-autocomplete/blob/master/src/autocomplete.js#L18

@NickColley
Copy link
Contributor

Hello @ediblecode,

I don't think we want to make the clientside rendering of this component render from the server as if it JavaScript fails it will not work for users.

For this reason the approach for server rendering on this component is described in the progressive enhancement section.

I would recommend making a small wrapper in your Gatsby application that renders the HTML markup described in that section server only, and replace it with the clientside version.

Let me know if this makes sense, I don't think we'd want to add more React specific complexity to this component at this time due to the time we have as a team.

I'll leave this open in case there's a simple thing we could consider that I've overlooked but we will close this later if there is not.

Thanks a lot for raising this :)

@ediblecode
Copy link
Author

@NickColley thanks for getting back to me.

I did see the progressive enhancement section but interestingly, our use-case is for autocomplete (typeahead) suggestions for search. We're progressively enhancing a search box with typehead suggestions dynamically loaded from the server as you type.

So although you're right:

I don't think we want to make the clientside rendering of this component render from the server as if it JavaScript fails it will not work for users.

in our case, it would work without JavaScript, it turns into a standard text box in a standard form, that does a GET request. (Forget the fact that I mentioned Gatsby, I know that wouldn't work without JS for a 'dynamic' SERP but you get the idea).

It looks like the component was designed with this sort of behaviour in mind because the source prop takes a function argument for retrieving results dynamically, calling populateResults.

I think that's a valid use case and the fix, from my investigations, to remove the client-specific code, looks fairly straightforward - shall I submit a PR and see what you think?

@ediblecode
Copy link
Author

Also, if you're interested, have a poke around https://github.com/nhsevidence/global-nav/tree/master/src/Header/Search/Autocomplete for where we're using it

@NickColley
Copy link
Contributor

Hey @ediblecode,

It looks like you're doing what I was thinking by rendering the fallback on the server.

Does the component still fail on the server despite it not being rendered?

I'd have thought those errors would only pop up if the server tried to render the clientside component...

@ediblecode
Copy link
Author

@NickColley it does. And it's actually not to do with the React-specific stuff, or rendering. Check out https://github.com/alphagov/accessible-autocomplete/blob/master/src/autocomplete.js#L18 for example. And the other issue is from Webpack (see my first comment for full description/links).

@NickColley
Copy link
Contributor

Ah so just by loading everything in it blows up since things like that are not within lifecycle methods, fair enough, I think we should guard those statements then as you've suggested. Do you want to do a pull request?

Would be interesting in a different piece of work to get some React specific guidance on how to implement the progressive enhancement as I imagine many people will try to render the component as-is server side.

@ediblecode
Copy link
Author

ediblecode commented Sep 13, 2019

Will do! And yeah, some extra React-specific guidance sounds like a good shout. Thanks for the help

@NickColley NickColley added the 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) label Sep 13, 2019
@36degrees 36degrees added this to the v2.0.3 milestone May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)
Projects
Development

Successfully merging a pull request may close this issue.

3 participants