Skip to content

Check if element has style attribute defined before reading/changing it#343

Merged
pocketjoso merged 2 commits into
pocketjoso:masterfrom
lucasrla:fix-element-style-undefined-v2
Mar 28, 2022
Merged

Check if element has style attribute defined before reading/changing it#343
pocketjoso merged 2 commits into
pocketjoso:masterfrom
lucasrla:fix-element-style-undefined-v2

Conversation

@lucasrla
Copy link
Copy Markdown
Contributor

Fix #342

Copy link
Copy Markdown
Owner

@pocketjoso pocketjoso left a comment

Choose a reason for hiding this comment

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

Just one comment - after that we can merge this. 👍


// but do that only if elements have their style attribute defined
// (ref: https://github.com/pocketjoso/penthouse/issues/342)
if (typeof element.style !== 'undefined') {
Copy link
Copy Markdown
Owner

@pocketjoso pocketjoso Mar 27, 2022

Choose a reason for hiding this comment

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

to save some computation since this code runs on a hot code path inside a loop (once for each selector in the css) - can you assign this typeof check to a variable and re-use it both in this if condition and in the one on line 35?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure!

Copy link
Copy Markdown
Owner

@pocketjoso pocketjoso 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!

@pocketjoso pocketjoso merged commit 765f485 into pocketjoso:master Mar 28, 2022
@RehanSaeed
Copy link
Copy Markdown

RehanSaeed commented May 21, 2022

Any chance of a release with this fix in it? Just hit this exact bug #342.

@pocketjoso
Copy link
Copy Markdown
Owner

pocketjoso commented May 21, 2022

Sure - I just published version 2.3.3!

@RehanSaeed
Copy link
Copy Markdown

You're awesome!

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.

TypeError: Cannot read property 'clear' of undefined

3 participants