check whether style needs a suffix with a set and remove debug casting #286
Conversation
🦋 Changeset detectedLatest commit: 3f21e88 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
JoviDeCroock
changed the title
| @@ -145,15 +145,6 @@ function _renderToString(vnode, context, isSvgMode, selectValue, parent) { | |||
| rendered = | |||
| rendered + | |||
| _renderToString(child, context, isSvgMode, selectValue, parent); | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
We fixed this in 10.13.0, I honestly think we shouldn't put a preact/debug fix in our hot path
There was a problem hiding this comment.
I'm a bit out of the loop. Which issue in preact/debug did this section work around?
There was a problem hiding this comment.
This one #257 this was fixed in Preact itself with preactjs/preact#3801
| @@ -338,6 +329,8 @@ function _renderToString(vnode, context, isSvgMode, selectValue, parent) { | |||
| } | |||
|
|
|||
| if (UNSAFE_NAME.test(type)) { | |||
| // this seems to performs a lot better than throwing | |||
| // return '<!-- -->'; | |||
There was a problem hiding this comment.
This could give us an additional 5% boost
There was a problem hiding this comment.
I really like that idea!
| @@ -1313,7 +1313,7 @@ describe('render', () => { | |||
| ); | |||
| } | |||
|
|
|||
| expect(render(<App />)).to.equal('<div><p>P481</p><p>P476951</p></div>'); | |||
| expect(render(<App />)).to.equal('<div><p>P0-0</p><p>P0-1</p></div>'); | |||
There was a problem hiding this comment.
use-id handling seems to be different in Preact 10.13
JoviDeCroock
force-pushed
the
optimize-style-suffix
branch
from
b5a749e to
fb3a5e9
Compare
| if ( | ||
| typeof val === 'number' && | ||
| // Exclude custom-attributes | ||
| !name.startsWith('--') && |
There was a problem hiding this comment.
Is this check doing the same thing as line 98, i.e. are they both trying to check for custom properties? Do we get any perf benefit by consolidating them?
In Preact >= 10.13.0 we fixed the debug casting, this removes this check as we're doing a major either way.
This also optimises style casting by using a Set, removing the need for the cache and thus saving some memory