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

(ssr) - create initial template for ssr plugin #400

Closed
wants to merge 3 commits into from
Closed

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Mar 7, 2021

Currently facing a few Windows issues but slowly getting there, this is a WIP

This is the code from #169

@changeset-bot
Copy link

changeset-bot bot commented Mar 7, 2021

⚠️ No Changeset found

Latest commit: ed2003e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2021

Size Change: +10 B (0%)

Total Size: 690 kB

Filename Size Change
packages/wmr/wmr.cjs 657 kB +10 B (0%)
ℹ️ View Unchanged
Filename Size Change
packages/wmr/demo/dist/about/index.html 657 B 0 B
packages/wmr/demo/dist/assets/Calendar.********.css 702 B 0 B
packages/wmr/demo/dist/assets/style.********.css 386 B 0 B
packages/wmr/demo/dist/chunks/class-fields.********.js 200 B 0 B
packages/wmr/demo/dist/chunks/compat.********.js 15.3 kB 0 B
packages/wmr/demo/dist/chunks/index.********.js 199 B 0 B
packages/wmr/demo/dist/chunks/prerender.********.js 2.45 kB 0 B
packages/wmr/demo/dist/class-fields/index.html 635 B 0 B
packages/wmr/demo/dist/compat/index.html 1.49 kB 0 B
packages/wmr/demo/dist/env/index.html 713 B 0 B
packages/wmr/demo/dist/error/index.html 646 B 0 B
packages/wmr/demo/dist/files/index.html 675 B 0 B
packages/wmr/demo/dist/index.********.js 7.07 kB 0 B
packages/wmr/demo/dist/index.html 703 B 0 B
packages/wmr/demo/dist/lazy-and-late/index.html 657 B 0 B

compressed-size-action

Comment on lines 43 to 49
if (typeof window !== 'undefined') {
if (document.querySelector('.app')) {
hydrate(<App />, document.body);
} else {
render(<App />, document.body);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary, given that hydrate is a wrapper that calls render if ! document.querySelector('script[type=isodata]')? (I suppose the answer is yes if script[type=isodata] isn't generated by the SSR method implemented in this PR, but in this case could this be clarified in a code comment, for the avoidance of doubt?)

Code reference:

let isodata = document.querySelector('script[type=isodata]');
// @ts-ignore-next
parent = parent || (isodata && isodata.parentNode) || document.body;
if (!initialized && isodata) {
hydrativeRender(jsx, parent);
} else {
render(jsx, parent);
}

The current documentation states:

https://github.com/preactjs/wmr/blob/main/packages/preact-iso/README.md#hydratejs

hydrate() is a thin wrapper around Preact's hydrate() method. It performs hydration when the HTML for the current page includes pre-rendered output from prerender(). It falls back to plain rendering in any other cases, which is useful if you're not pre-rendering during development. This method also checks to make sure its running in a browser context before attempting any rendering - if not, it does nothing.

Comment on lines +43 to +45
if (typeof window !== 'undefined') {
hydrate(<App />, document.body);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will probably see this in PR #403 ( #403 (review) ) but just in case this goes under the radar for this PR #400, here it is again:

The typeof window !== 'undefined' browser check can be removed.

References:

if (typeof window === 'undefined') return;

https://github.com/preactjs/wmr/blob/main/packages/preact-iso/README.md#hydratejs

This method also checks to make sure its running in a browser context before attempting any rendering - if not, it does nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants