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

style: use functions as style key creation mechanism #116

Merged
merged 3 commits into from
Jan 20, 2022

Conversation

jchavarri
Copy link
Collaborator

Fixes #112.

Just as an example, in the prod bundle of example demo app, size goes from 115KB to 103KB 😅

Based on the ideas discussed in #113, and @glennsl additions to poc.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@jchavarri jchavarri requested a review from davesnx January 20, 2022 08:46
Copy link
Member

@davesnx davesnx left a comment

Choose a reason for hiding this comment

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

Love the approach. Improves the API, using an array is substantially better than labelled arguments for this use-case. No need to bring functions such as merge styles or combine.

Reduces the bundle size by removing the 0s, but does it add all these functions into the resultant bundle?

lib/dom.mli Outdated
Comment on lines 108 to 110
type style_key

type t = style
Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to use proper CSS terminology, style_key would be a "declaration" ("decl" for short), and style would be a "declaration block".

lib/dom.mli Outdated

type t = style

let string_style_key key value =
Copy link
Contributor

Choose a reason for hiding this comment

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

And the "key" part of the declaration is called a "property", so it would perhaps be more accurate to call this string_style_prop, at least in the way it's being used, partially applied.

@jchavarri
Copy link
Collaborator Author

@glennsl @davesnx thanks for the review :) I updated the names to be closer to CSS semantics.

but does it add all these functions into the resultant bundle?

No, only what's used. js_of_ocaml is quite efficient in terms of dead-code optimization.

@jchavarri jchavarri merged commit df73442 into main Jan 20, 2022
@jchavarri jchavarri deleted the efficient-dom-style branch January 20, 2022 22:19
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.

Make React.Dom.Style.make more bundle size friendly
3 participants