fix: Default document rendering

[Meemaw]

What kind of change does this PR introduce?

Fix default document rendering.

Fixes #154

What is the current behaviour?

When useDocument=false we didn't properly render the Next.JS document (we skipped it) which meant that any SEO elements defined in custom _app or in page were not rendered.

This is wrong behaviour that is not in line with useApp where this flag only controls if we should use custom _app component, but not changing the rendering flow.

What is the new behaviour?

useDocument=false is now inline with the useApp=false behaviour and document rendering flow is always correct.

This change also simplifies the code and lets us get rid of some weird conditions.

Does this PR introduce a breaking change?

What changes might users need to make in their application due to this PR?

Other information:

Please check if the PR fulfills these requirements:

• Tests for the changes have been added
• Docs have been added / updated

[coveralls]

Coverage remained the same at 100.0% when pulling 2a2ad98 on fix-default-document-rendering into db4045d on master.

[Meemaw]

This is exactly how useApp behaves.

[toomuchdesign]

Hi @Meemaw, thanks for this PR and all the maintenance job during this days ❤️ .

I still haven't had the opportunity of fully digging into this pr. I'll do as soon as possible. Does it actually concern and fix #154?

At a first glance I'd expect this library to not render the head element when useDocument === false and consequently skipping and nested SEO element.

🙌

[Meemaw]

Hi @Meemaw, thanks for this PR and all the maintenance job during this days ❤️ .

I still haven't had the opportunity of fully digging into this pr. I'll do as soon as possible. Does it actually concern and fix #154?

Actually it seems that the issue is in the setup (perhaps missing jest patch) and does not require any changes on our side -- the reproducer works in our repo.

At a first glance I'd expect this library to not render the head element when useDocument === false and consequently skipping and nested SEO element.

Hmm 🤔 I'm actually not sure which one I would prefer. On one hand I see many advantages to doing full Next.JS render even when useDocument=false:

• Simplified code flow internally / less branches to worry about
• Possibility to test page's SEO elements even when useDocument=false e.g. page title takes precedence over _document's title anyway and this is the element users care about in most of the cases as it differs between tests/routes
• More closely aligned with the main selling point of the library: "The idea behind this library is to reproduce as closely as possible the way Next.js works without spinning up servers, and render the output in a local JSDOM environment."

From what I see the main point of even preserving useApp and useDocument flags is possibility to avoid mocking complexity they might bring. E.g. in complex apps custom _document  might do 20 BE requests that would have to be mocked for every test.

How do you see it?

[toomuchdesign]

Hi @Meemaw,
I fully agree with your point of view about useApp and useDocument flags. I alse tend to see useDocument as a possible future escape hatch in case Next.js rewrites the way Document works and we were not able to make work it here.

Even without document, this library could be useful in quite a lot projects. Especially simple ones for which browser testing would be overkill or too expensive.

[toomuchdesign]

If we went this direction we might consider taking an alternative testing strategy since I feel like we're exposing a lot of Next.js' implementation details as test expectation.

I currently don't have the full picture of our document tests, may we now have a bit of redundancy?

BTW, I like the way this approach simplify our code. Probably the cost of it gets reflected onto tests.

If you agree I'd keep the decision for after the upcoming release. We're currently tackling quite a few issues which I'd like to see fixed before proceeding further. In the meanwhile let's keep talking. 👍

[toomuchdesign]

Should we consider this output (along with next-head-count meta, data-n-css noscript..) implementation details not depending on our code and keep them out from out expectations?

If Next.js added a new next-tag in a minor version, our test would get red, wouldn't them?