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

Fix hydration by not inserting hydration keys in <head> #137

Merged
merged 6 commits into from
May 10, 2022

Conversation

lukechu10
Copy link
Contributor

TODO: this currently uses a hack that I'm not so proud of. I'll probably need to release a new version of sycamore to do this properly but at least the problem is fixed.

The problem with hydration was that hydration keys were rendered in the head strings. This would cause a conflict when hydrating because there would be multiple nodes with the same hydration keys. That, in turn, is because there are multiple calls to render_to_string, each which creates a new "hydration context".

Note that this is a PR against the feat-sycamore-0.4.0 branch instead of the main branch.

@arctic-hen7
Copy link
Member

Neat, thanks! If you don't mind, I'd like to roll back all the insane Signal stuff I've done before merging this.

@lukechu10
Copy link
Contributor Author

Yeah no problem. Also there probably should be a function directly in sycamore to render to a string without hydration keys and hydration markers.

@lukechu10 lukechu10 marked this pull request as ready for review April 30, 2022 23:58
@lukechu10
Copy link
Contributor Author

I released sycamore v0.8.0-beta.5 which allowed me to remove the workaround. This should be ready now.

Copy link
Member

@arctic-hen7 arctic-hen7 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks so much!

One note: it might be good to alter the docs on use_context_or_else in Sycamore, I had no idea it actually added the result of the closure to the context.

@arctic-hen7 arctic-hen7 merged commit 990de00 into framesurge:feat-sycamore-0.4.0 May 10, 2022
arctic-hen7 added a commit that referenced this pull request May 10, 2022
These were just for the demos that weren't ready at the time of the PR.
@lukechu10 lukechu10 deleted the fix-hydration branch May 10, 2022 15:58
@lukechu10
Copy link
Contributor Author

One note: it might be good to alter the docs on use_context_or_else in Sycamore, I had no idea it actually added the result of the closure to the context.

Yeah, I should probably make that clearer in the docs. Didn't realize it might have meant something else.

arctic-hen7 added a commit that referenced this pull request May 30, 2022
* chore: updated versions and editions

Sycamore v0.8 only runs on Rust 2021.

* fix: fixed some rust 2021 syntax issues

* fix: fixed easy errors in `perseus` crate

Still have to do:
- `router/`
- `template/`
- `shell.rs`
- One tricky error in `error_pages.rs`

* fix: fixed all errors in `perseus` crate

I removed the troublesome function in `error_pages.rs`, so that may bite
me soon.

* fix: fixed clippy lints

* fix: fixed macros and made `basic` example work

* fix: partial fixes for `rx_state`

Still have problems with `bind:value` (as we will in all the state
platform examples for now).

* fix: updated state examples (errors persist)

The remaining errors will be fixed after `make_rx` can work with
non-`Rc` `Signal`s.

* fix: updated all examples (lifetime errors persist)

* fix: fixed engine

Nothing works without hydration disabled though...

* refactor: made renderers use top-level router context

This *should* make the state platform work with lifetimes.

* feat: rewrote render context to use `Signal`s

* fix: made macros work with the new render context logic

* fix: updated unreactive macros and example

* chore: tmp commit before rollback

* revert: revert to before axing `RcSignal`s

I have been shown a much better way of achieving the same outcome.

* revert: return to previous changes

It will be easier to manually undo changes to make sure we preserve some
good things.

This reverts commit 15187b5.

* feat: moved back to `RcSignal`s

This avoids a huge number of lifetime issues, and actually ends up being
more performant, without compromising on ergonomics.

* feat: made `struct` given to user's template use `&'a RcSignal<T>`

This should make Perseus several orders of magnitude more ergonomic, in
line with Sycamore's new no-clones system!

* fix: fixed global state functionality in the macros

This requires an irritating change to import practices unfortunately,
but the convenience and ergonomics are worth it.

* fix: fixed lifetimes errors in all examples

* fix: fixed all lifetimes issues

This also involved some minor changes to the macros to fix some nested
state issues.

* fix: fixed nested state references

This improves ergonomics and makes the auth example compile.

* fix: fixed hydration by not inserting hydration keys in `<head>` (#137)

* refactor: simplify provide_context_signal_replace

Also slightly improves performance in only making a single call to use_context

* fix: do not insert hydration keys in the head string

* chore: remove perseus/hydrate feature from Cargo.toml

* chore: merge imports for consistent code style

* fix: update sycamore to v0.8.0-beta.5 and remove workaround

Co-authored-by: arctic_hen7 <arctic_hen7@pm.me>

* chore: updated deps after #137

These were just for the demos that weren't ready at the time of the PR.

* chore: re-added `hydrate` feature to `basic` example

Hydration still doesn't work in the `auth` example.

* chore: removed unused dep

* fix: fixed doc tests issue

* fix: fixed naming of `PerseusRoot` (was wrongly `perseus_root`)

* feat: updated `index_view` example

* chore: updated to latest sycamore beta

This should fix the issues with the `body` element.

* fix: fixed an imports issue with latest sycamore beta

* fix: fixed types to make i18n work

* test: fixed `rx_state` tests for slightly updated structure

* fix: ignored a failing doctest

* docs: updated security.md for next beta version

* docs: added new docs for v0.4.x

Also locked the old v0.3.4-5 docs to a specific commit hash, which keeps
the examples there safe to use.

Co-authored-by: Luke Chu <37006668+lukechu10@users.noreply.github.com>
arctic-hen7 added a commit that referenced this pull request Jun 2, 2022
* chore: updated versions and editions

Sycamore v0.8 only runs on Rust 2021.

* fix: fixed some rust 2021 syntax issues

* fix: fixed easy errors in `perseus` crate

Still have to do:
- `router/`
- `template/`
- `shell.rs`
- One tricky error in `error_pages.rs`

* fix: fixed all errors in `perseus` crate

I removed the troublesome function in `error_pages.rs`, so that may bite
me soon.

* fix: fixed clippy lints

* fix: fixed macros and made `basic` example work

* fix: partial fixes for `rx_state`

Still have problems with `bind:value` (as we will in all the state
platform examples for now).

* fix: updated state examples (errors persist)

The remaining errors will be fixed after `make_rx` can work with
non-`Rc` `Signal`s.

* fix: updated all examples (lifetime errors persist)

* fix: fixed engine

Nothing works without hydration disabled though...

* refactor: made renderers use top-level router context

This *should* make the state platform work with lifetimes.

* feat: rewrote render context to use `Signal`s

* fix: made macros work with the new render context logic

* fix: updated unreactive macros and example

* chore: tmp commit before rollback

* revert: revert to before axing `RcSignal`s

I have been shown a much better way of achieving the same outcome.

* revert: return to previous changes

It will be easier to manually undo changes to make sure we preserve some
good things.

This reverts commit 15187b5.

* feat: moved back to `RcSignal`s

This avoids a huge number of lifetime issues, and actually ends up being
more performant, without compromising on ergonomics.

* feat: made `struct` given to user's template use `&'a RcSignal<T>`

This should make Perseus several orders of magnitude more ergonomic, in
line with Sycamore's new no-clones system!

* fix: fixed global state functionality in the macros

This requires an irritating change to import practices unfortunately,
but the convenience and ergonomics are worth it.

* fix: fixed lifetimes errors in all examples

* fix: fixed all lifetimes issues

This also involved some minor changes to the macros to fix some nested
state issues.

* fix: fixed nested state references

This improves ergonomics and makes the auth example compile.

* fix: fixed hydration by not inserting hydration keys in `<head>` (#137)

* refactor: simplify provide_context_signal_replace

Also slightly improves performance in only making a single call to use_context

* fix: do not insert hydration keys in the head string

* chore: remove perseus/hydrate feature from Cargo.toml

* chore: merge imports for consistent code style

* fix: update sycamore to v0.8.0-beta.5 and remove workaround

Co-authored-by: arctic_hen7 <arctic_hen7@pm.me>

* chore: updated deps after #137

These were just for the demos that weren't ready at the time of the PR.

* chore: re-added `hydrate` feature to `basic` example

Hydration still doesn't work in the `auth` example.

* chore: removed unused dep

* fix: fixed doc tests issue

* fix: fixed naming of `PerseusRoot` (was wrongly `perseus_root`)

* feat: updated `index_view` example

* feat: added axum integration

This is all untested as yet, but everything *should* work.

* chore: updated to latest sycamore beta

This should fix the issues with the `body` element.

* fix: fixed an imports issue with latest sycamore beta

* fix: fixed types to make i18n work

* test: fixed `rx_state` tests for slightly updated structure

* fix: ignored a failing doctest

* docs: updated security.md for next beta version

* docs: added new docs for v0.4.x

Also locked the old v0.3.4-5 docs to a specific commit hash, which keeps
the examples there safe to use.

* feat: integrated axum with other integrations

There are still some issues with shared state though that make this
completely unusable.

* fix: fixed shared state issues

No clue why using extensions didn't work, but now we're using closure
captures, which are compile-time checked anyway.

* fix: made static content work

Just some simple errors made this fail. Hopefully, all tests should now pass...

Co-authored-by: Luke Chu <37006668+lukechu10@users.noreply.github.com>
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