Make React bundle work server-side

[andymsuk]

Overview

Fixes #352, so that the component can be used in a server-side React environment.

Changes

• Set globalObject to 'this' in the React Webpack config, so that it doesn't have unwanted references to window, in order for it to work in a server-side environment
• Check navigator is defined before using it, in order for it to work in a server-side environment
• Only include the role of combobox once the component has mounted, so that the server-side rendered markup (i.e. when the user has JavaScript turned off) has no role attribute, and so it's role defaults back to textbox
• Only include the aria hint (aria-describedby) once the component has mounted, so that the server-side rendered markup (i.e. when the user has JavaScript turned off) has no aria-describedby as it doesn't make sense if there the JavaScript is not running.

[NickColley]

Thanks a lot for helping with this.

One thing that worries me is this change:

Only include the role of combobox once the component has mounted, so that the server-side rendered markup (i.e. when the user has JavaScript turned off) has no role attribute, and so it's role defaults back to textbox

As we discussed in the issue, we probably want a completely different component to be rendered server side. We want people to send the markup from the progressive enhancement section to users before loading the JavaScript, instead of sending a variation of the enhanced component.

What're your thoughts on ensuring that we don't encourage folks rendering a broken experience server side?

[NickColley]

If you look at their example will give more context:

https://github.com/nice-digital/global-nav/blob/master/src/Header/Search/Autocomplete/Autocomplete.jsx#L133

[NickColley]

Perhaps some guidance in the README explaining server rendering could help.

[ediblecode]

Thanks for looking at this. Must apologize though, been meaning to look at this for aaaages after I raised the issue! Good timing though as I'm picking the work up again next week.

For what it's worth I agree with the comment about "we probably want a completely different component to be rendered server side" so I think this fix should probably just be all the bits that break loading this component in a Node environment.

[andymsuk]

Hi @NickColley and @ediblecode

Thanks for your comments.

With regards to wanting a different component rendered server-side, I have a couple of thoughts.

My first thought is that it really depends on your use case for this component as to what you'd want rendered server-side for the case where JS is turned off or not loading for whatever reason. For example, in my personal use case, I want to use this component on my app merely as a way to suggest values to the user, so that they don't have to type the full value every time. In this case, just returning a default textbox type of input would be suitable. However, I appreciate that another use case is that the user is obliged to pick a value from the component, in which case I'd agree that a select is a more suitable fallback.

My second thought is that if you're rendering a select server-side and then switching it out client-side for an input type="text" then, unless you've got your CSS spot on (and even still possibly in some browsers) the user will see a "flash" as the component changes. This "flash" effect may not be an option for some people using this component.

As you both suggested, I think the server-side fallback could do with some more thought, possibly having multiple options to account for the use cases I outlined above. I've therefore removed the last commit around changing the server-side fallback, so that this can be addressed either directly by the person using the component in their app, or by us or anyone else in a future enhancement PR. At least this way, as @ediblecode says, this fix can just be the bits that break loading this component in a Node environment.

[NickColley]

@andymsuk thanks for updating that, it's looking good I think the only last things to do are:

• tidy up the commits
• add a CHANGELOG entry

I can do this on your behalf if you want, let me know.

[andymsuk]

Thanks @NickColley.

I've tidied up the commits and added to the CHANGELOG, but feel free to edit/tidy more if you'd like.

[NickColley]

This makes sense to me, thanks for the tidy, CHANGELOG looks great too.

@ediblecode would you mind giving this a review too please?

[ediblecode]

Looks good to me, have approved. Thanks @NickColley and @andymsuk

[36degrees]

Looks good to me 👍It'd be good to update the PR description to strike-through the changes that are no longer included – I was originally looking for those changes in the files changed and it took me a few minutes to read through the conversation and work out what was going on.