-
Notifications
You must be signed in to change notification settings - Fork 22
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
Update to yew 0.20 #69
Conversation
Heads-up, the code in this PR no longer compiles, it gives this error:
Using yew master on rev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM.
I will line up a release tomorrow for 0.11.
Thanks.
//! ## Features Flags | ||
//! | ||
//! - `macros`: Enabled by default, this flag enables procedural macro support. | ||
//! - `random`: Enabled by default, this flag uses `fastrand` crate to generate a random class name. | ||
//! Disabling this flag will opt for a class name that is counter-based. | ||
//! - `parser`: Disabled by default, this flag enables runtime parsing of styles from strings. You |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parser is actually enabled by default.
I am indifferent in disabling or enabling this by default.
I felt disabling this might be better but we do lose debug assertions on interpolation values. We can make the parser by default debug only, but there doesn't seem to be a way to exclude dependencies on release anyways so the benefit seems not big enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm you are right. Can we maybe internally consume stylist-core/parser
(trusting the tree shaking and dead code elimination in release mode) but still only publicly expose the additional features with stylist/parser
turned on?
The point being that the default might trick users into using the runtime parsing via the IntoPropValue
impls, which causes unnecessary overhead.
EDIT: I see, power users wouldn't be able to get rid of the nom
dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split the feature into parser
for the public part and debug_parser
for the debug assertions. Note that there were a few examples that mistakenly used the parser/runtime parsed styles in their <Global />
styles, and disabling this shaves some kB from the wasm binary ;)
This PR tracks the changes necessary to update to the eventual yew 0.20 release.
Upgrades:
#[hook]
on hook integrationRenderer
andfeatures = ["csr"]
in the examplesCloses #87
Closes #90