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.

Sign up for GitHub

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

Jump to bottom

Don't call CSSStyleDeclaration.setProperty for style strings #1394

Merged
merged 2 commits into from Mar 11, 2019
Merged

Don't call CSSStyleDeclaration.setProperty for style strings #1394

merged 2 commits into from Mar 11, 2019

Conversation

dreamofabear
Copy link

Avoids unnecessary calls to setProperty for style strings.

  • Browsers and JSDOM will ignore these but workerDOM (currently) doesn't since it lacks a property whitelist.
  • Minor perf boost by avoiding loop of value.length and some regex ops.

/cc @kristoferbaxter

@coveralls
Copy link

coveralls commented Mar 11, 2019 •
edited

Coverage Status

Coverage remained the same at 100.0% when pulling 9723847 on choumx:css-setProperty-strings into 3c821b3 on developit:master.

marvinhagemeister
marvinhagemeister approved these changes Mar 11, 2019
View reviewed changes
Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Great catch, and an awesome first PR 💯 Thanks for your contribution 🎉

@marvinhagemeister marvinhagemeister merged commit b958217 into preactjs:master Mar 11, 2019
@dreamofabear dreamofabear deleted the css-setProperty-strings branch March 11, 2019 22:01
@developit
Copy link
Member

Wow, nice catch! This one was my bad 🥺

@marvinhagemeister marvinhagemeister mentioned this pull request Mar 20, 2019
Merged
marvinhagemeister added a commit that referenced this pull request Mar 20, 2019
@marvinhagemeister
Don't iterate over old css string (#1429)
193d97e 
This PR fixes an issue where we did iterate over each character of the previous style value. This only happened when the previous value was of type `string`. See the debug logs here: https://travis-ci.org/developit/preact/builds/50882293

Related to #1394
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marvinhagemeister marvinhagemeister marvinhagemeister approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dreamofabear @coveralls @developit @marvinhagemeister