Assert there are no circular references #3025
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the check is costly in release builds and should always fail note that the current PartialEq impl is *not* symmetric! should be fixed as well, with an improved design
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Visit the preview URL for this PR (updated for commit 111cb96): https://yew-rs-api--pr3025-perf-node-link-1ppwmv1h.web.app (expires Sat, 17 Dec 2022 13:49:17 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Benchmark - SSRYew Master
Pull Request
|
Size Comparison
✅ None of the examples has changed their size significantly. |
wasm_bindgen does not yet support #[should_panic] see also rustwasm/wasm-bindgen#2286
ranile
previously approved these changes
Dec 10, 2022
|
Yeah locally it does. Something is really wrong with the github actions benchmark setup:
|
WorldSEnder
dismissed stale reviews from ranile and GitHub Actions
via
111cb96
|
I have removed a test for the |
|
Can the link method be removed entirely? |
|
Not that I know how. It's important for empty and nested components' to be able to forward their |
ranile
approved these changes
Dec 10, 2022
WorldSEnder
added a commit
to WorldSEnder/yew
that referenced
this pull request
Jan 10, 2023
* assert there are no circular references the check is costly in release builds and should always fail note that the current PartialEq impl is *not* symmetric! should be fixed as well, with an improved design * remove internal test for cyclic node refs wasm_bindgen does not yet support #[should_panic] see also rustwasm/wasm-bindgen#2286
WorldSEnder
added a commit
that referenced
this pull request
Jan 19, 2023
* assert there are no circular references the check is costly in release builds and should always fail note that the current PartialEq impl is *not* symmetric! should be fixed as well, with an improved design * remove internal test for cyclic node refs wasm_bindgen does not yet support #[should_panic] see also rustwasm/wasm-bindgen#2286
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The removed check is costly in release builds and should always fail. Note that the current PartialEq impl of
NodeRefis not symmetric, it's useful only to check for circular dependencies! This should be fixed as well, with an improved design.Fixes #3024
Checklist