feature/RHTL-68 – server-side-rendering

[joshuaellis]

What:

New feature SSR testing – breaking change.
Split the repo into dom|native|server

dom: react-dom
native: react-test-renderer
server: react-dom

Why:

#68

How:

RTR does not currently support SSR, react-dom does, this will be the default renderer for SSR testing. However, the plan is to expose a custom renderer usage where users can use other renderers e.g preact. User's will have their test engine auto

Checklist:

• Documentation updated
• Tests
• Ready to be merged
  
• Added myself to contributors table
  

[joshuaellis]

This isn't ready but i've made the move over into 'native' and thought it'd be good to track along side #498 as that is also a huge change to the codebase.

[joshuaellis]

the .eslintrc config is not being picked up hence the lint errors. Also I can't actually run validate or lint because of the following error

Error: .eslintrc » ./node_modules/kcd-scripts/eslint.js » /Users/joshua/code/react-hooks-testing-library/node_modules/kcd-scripts/node_modules/eslint-config-kentcdodds/jest.js:
        Environment key "jest/globals" is unknown

My assumption this is related to #502 any ideas @marcosvega91?

[marcosvega91]

Hi @joshuaellis have you reinstalled all modules ? Are you using yarn or npm?.

If you are using yarn remember to remove your local yarn.lock then Delete node_modules folder and install all libraries again. Let me know if this solve your problem

[joshuaellis]

Hi @marcosvega91,

If you are using yarn remember to remove your local yarn.lock then Delete node_modules folder and install all libraries again.

I was using yarn. But doing the above method gave the same error. repeating steps above and re-installing with npm worked fine. Does kcd-scripts not support yarn? Weird if not.

[marcosvega91]

You're right. I'm receiving the same error. I'll try to understand why with yarn is giving this problem. Thanks for raising it 😄

[marcosvega91]

The problem with yarn is that is not resolving correctly eslint. yarn is using 6.8.0 from gatsby will it should use the version 7.15.0 from kcd-scripts

[joshuaellis]

Hi @mpeyper,

I've just migrated the previous server work you did & the server/suspenseHook fails because -

ReactDOMServer does not yet support Suspense.
      31 |       renderProps = props
      32 |       baseAct(() => {
    > 33 |         const serverOutput = ReactDOMServer.renderToString(toRender(props))
         |                                             ^
      34 |         container.innerHTML = serverOutput
      35 |       })
      36 |     },

I just wanted to know if you remember this happening to you or not? IMO i can't initially see why this test would fail as we're not using suspense. Maybe i'm not understanding the error well enough. If you're not sure dw.

[mpeyper]

@joshuaellis Yes, this was an issue for me too.

[mpeyper]

@joshuaellis the TS migration (#498, #520) is in a beta release now. I suggest we rebase these changes on the [beta branch(https://github.com/testing-library/react-hooks-testing-library/tree/beta) before continuing.

[joshuaellis]

Great stuff @mpeyper thanks for letting me know.

[joshuaellis]

I've managed to navigate alot of the Typescript stuff so far, I'm really struggling how to solve the issue in native/pure with the toRender –

const toRender = (props?: TProps): JSX.Element => (
    <Wrapper {...props}>
      <TestHook hookProps={props} {...testHookProps} />
    </Wrapper>
  )

the issue i'm getting is

Type '{ children: Element; }' is not assignable to type 'TProps'.
'TProps' could be instantiated with an arbitrary type which could be unrelated to '{ children: Element; }'.

I was wondering if it had something to do with this on going issue – DefinitelyTyped/DefinitelyTyped#34237 if anyone has ideas, they're definitely welcome.

[joshuaellis]

I've managed to navigate alot of the Typescript stuff so far, I'm really struggling how to solve the issue in native/pure with the toRender –

const toRender = (props?: TProps): JSX.Element => (
    <Wrapper {...props}>
      <TestHook hookProps={props} {...testHookProps} />
    </Wrapper>
  )

the issue i'm getting is

Type '{ children: Element; }' is not assignable to type 'TProps'.
'TProps' could be instantiated with an arbitrary type which could be unrelated to '{ children: Element; }'.

I was wondering if it had something to do with this on going issue – DefinitelyTyped/DefinitelyTyped#34237 if anyone has ideas, they're definitely welcome.

DM, it actually had to do with this issue microsoft/TypeScript#28938 (comment)

[mpeyper]

I updated the base branch to reduce the noise in the review and I think we'll want this to honour as a beta first anyway.

I've had a brief look on my phone just now and I like where this is headed. I have a few comments to leave, but I'll wait until I'm on my laptop to do so.

[joshuaellis]

Stil to do this comment & define some tests for ‘src/pure’

[mpeyper]

Stil to do this comment & define some tests for ‘src/pure’

I've fixed the types for the submodules now and I've removed the custom module (but left the generics as discussed in that comment).

I worked around the type issue by generating the submodules instead of having static files for them. In some ways I like this solution more, but I'm still a bit upset I couldn't figure out the TS configuration to get it to work.

As for tests of the root module, I'm still not sure how to do this as we have both dependencies installed so testing the error cases or even getting it to select the dom renderer is going to be a challenge. I'm not a fan of exporting functions just for testing, but it might be the only way for this one? I'm also happy to put it on the back burner for now. The logic is fairly straight forward and unlikely to change much in the future.

Other than that, I think it's just the docs that need some love on this one now.

[mpeyper]

I think we should release this into a beta now that the previous beta has been released. What do you think @joshuaellis?

[joshuaellis]

Absolutely, let’s push to beta. I can begin work on the docs later today 💪🏼

[github-actions]

🎉 This PR is included in version 5.0.0-beta.1 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[mpeyper]

hmmmm... I was expecting that to publish as 5.0.0-beta.1. The bot even picked up the BREAKING CHANGES in the release tag?

[joshuaellis]

bad bot. That is weird, I would have expected that too.

[marcosvega91]

to trigger the release of the 5 beta I guess you should merge master into beta first and then merge the PR into the beta branch.

Your master branch has a commit Merge branch 'beta' that is linked to the 4 version.

Anyway BREAKING CHANGE is read by semantic release and considered as a major release

[mpeyper]

Thanks @marcosvega91, I have rebuilt the beta branch no with the 4.0.0 tag in the history so the next build should be right 🤞

[mpeyper]

I just tried the beta out in one of my own projects. All renderers are working great and the auto-detection is also working. The only issue so far is that when niether react-dom or react-test-renderer are installed, the error message is coming out as

Error: Could not auto-detect a React renderer. Are you sure you've installed one of the following
  - react-dom/n  - react-test-renderer

Whoops. I'll fix that quickly and get a new build out.