Conversation
f6bb8b5 to
3337fe0
Compare
tofumatt
left a comment
There was a problem hiding this comment.
Looks good to me, but curious about the removal of wp-blob from the wp_register_script call.
| wp_register_script( | ||
| 'wp-utils', | ||
| gutenberg_url( 'build/utils/index.js' ), | ||
| array( 'lodash', 'wp-blob', 'wp-deprecated', 'wp-api-request', 'wp-i18n', 'wp-keycodes' ), |
There was a problem hiding this comment.
Why is blob removed here? Is that intentional/related to this change?
There was a problem hiding this comment.
It should be removed in one of the previous PRs, it's no longer used inside.
There was a problem hiding this comment.
I forgot to mention. Sorry about it.
There was a problem hiding this comment.
Ah okay, just wasn't sure it was related. 👍
packages/entities/README.md
Outdated
| @@ -0,0 +1,13 @@ | |||
| # @wordpress/entities | |||
|
|
|||
| Entities utils for WordPress. | |||
There was a problem hiding this comment.
Can we at least say "keyboard entities" or something? 😄
There was a problem hiding this comment.
Should we call it html-entities?
There was a problem hiding this comment.
Those are HTML entities, btw :)
There was a problem hiding this comment.
Oh geez, that makes more sense. I think I read the TAB import change at the top of the PR diff and my brain got confused. Time for more ☕️
"HTML Entities" would be great.
tofumatt
left a comment
There was a problem hiding this comment.
Looks good after the blob explanation. I'd tweak the README for the new module, but otherwise 👍
|
It looks like we introduced regression in e2e tests earlier today when merging changes that introduce |
|
There are a few possibilities in regards to the package name:
@aduth, @tofumatt, @youknowriad any thoughts? |
|
|
|
Related: #7824 WordPress/packages#12 (specifically WordPress/packages#12 (comment) ) Was a point that it could be included in the proposed |
|
Let's go with |
|
Prior art: The And in case anyone is tempted to lodge the NIH complaint: Take a look at the built size of |
|
http://php.net/manual/en/function.html-entity-decode.php - this will match with PHP version of similar functionality. |
82.8 KB :) Good to know about that in case we would need something working in Node or RN env. |
Yes, in fact it was implemented this way in Calypso for Node vs. browser: https://github.com/Automattic/wp-calypso/blob/master/client/lib/formatting/decode-entities/browser.js I'm not entirely sure it's great to have the separate implementations. From what I recall, the browser-based implementation (at least without shortcutting/caching) is quite non-performant as well (Automattic/wp-calypso#9725). |
Description
Part of #3955.
This PR extracts new
@wordpress/entitiespackage which contains one utility methoddecodeEntities.I also added 2 unrelated changes:
.npmrcfiles to a few packages@wordpress/keycodesHow has this been tested?
npm testand make sure there are no regressions.wp.utils.decodeEntitiesshould work as before but include warning informing about planned deprecation, e.g.