Skip to content

Fix className patch not applied to props#2279

Merged
marvinhagemeister merged 1 commit into
masterfrom
fix-classname-normalization
Jan 26, 2020
Merged

Fix className patch not applied to props#2279
marvinhagemeister merged 1 commit into
masterfrom
fix-classname-normalization

Conversation

@marvinhagemeister

@marvinhagemeister marvinhagemeister commented Jan 25, 2020

Copy link
Copy Markdown
Member

This error was caused by our className patch being applied to the wrong props reference. Whenever a component is in play like with most CSS-in-JS solutions and we attempt to normalize props we'll create a new reference.

preact/compat/src/render.js

Line 111 in 8164289

attrs = vnode.props = {};

A few lines later we attached the className patch, but we didn't update the reference of our props variable to point to the new object.

Since the order of our normalizations isn't important, the easiest fix is to move the className check before the other where props will be assigned a new variable.

Fixes #2275

@github-actions

github-actions Bot commented Jan 25, 2020 •
edited
Loading

Copy link
Copy Markdown

Size Change: -1 B

Total Size: 38.2 kB

Filename Size Change
compat/dist/compat.umd.js 3 kB -1 B
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 2.95 kB 0 B
compat/dist/compat.module.js 2.97 kB 0 B
debug/dist/debug.js 3.08 kB 0 B
debug/dist/debug.module.js 3.07 kB 0 B
debug/dist/debug.umd.js 3.14 kB 0 B
devtools/dist/devtools.js 175 B 0 B
devtools/dist/devtools.module.js 185 B 0 B
devtools/dist/devtools.umd.js 252 B 0 B
dist/preact.js 3.69 kB 0 B
dist/preact.min.js 3.69 kB 0 B
dist/preact.module.js 3.71 kB 0 B
dist/preact.umd.js 3.75 kB 0 B
hooks/dist/hooks.js 1.06 kB 0 B
hooks/dist/hooks.module.js 1.08 kB 0 B
hooks/dist/hooks.umd.js 1.12 kB 0 B
test-utils/dist/testUtils.js 390 B 0 B
test-utils/dist/testUtils.module.js 392 B 0 B
test-utils/dist/testUtils.umd.js 469 B 0 B

compressed-size-action

@coveralls

coveralls commented Jan 25, 2020 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.1%) to 99.807% when pulling dfc7f36 on fix-classname-normalization into 5d85cad on master.

JoviDeCroock
JoviDeCroock approved these changes Jan 25, 2020
View reviewed changes

@JoviDeCroock JoviDeCroock left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great find! This should also fix #2224, no?

@marvinhagemeister

marvinhagemeister commented Jan 26, 2020

Copy link
Copy Markdown
Member Author

@JoviDeCroock unfortunately not, but I think I know what's going on.

@marvinhagemeister marvinhagemeister force-pushed the fix-classname-normalization branch from 65c4ca7 to dfc7f36 Compare January 26, 2020 07:32
@marvinhagemeister marvinhagemeister merged commit 14d8523 into master Jan 26, 2020
@marvinhagemeister marvinhagemeister deleted the fix-classname-normalization branch January 26, 2020 07:37
porfirioribeiro pushed a commit to porfirioribeiro/preact that referenced this pull request Feb 3, 2020
@marvinhagemeister @porfirioribeiro
Fix className patch not applied to props (preactjs#2279)
898c69b
@snyk-bot snyk-bot mentioned this pull request Mar 8, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JoviDeCroock JoviDeCroock JoviDeCroock 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.

Preact 10.1.0 breaks Emotion styled components

3 participants

@marvinhagemeister @coveralls @JoviDeCroock