ui-utils: Add new package with two type checking functions #28028
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,10 @@ | |
|
|
||
| ## Unreleased | ||
|
|
||
| ### New Features | ||
|
|
||
| - Add `is` utility | ||
|
|
||
| ## 3.4.0 (2019-06-12) | ||
|
|
||
| ### New Features | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| package-lock=false |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <!-- Learn how to maintain this file at https://github.com/WordPress/gutenberg/tree/master/packages#maintaining-changelogs. --> | ||
|
|
||
| ## Unreleased | ||
|
|
||
| Initial release. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # UI Utils | ||
|
|
||
| A collection of utilities useful for building user interfaces. | ||
|
|
||
| ## API | ||
|
|
||
| <!-- START TOKEN(Autogenerated API docs) --> | ||
|
|
||
| <a name="isDefined" href="#isDefined">#</a> **isDefined** | ||
|
|
||
| Checks whether a value is defined, asserting that it is neither | ||
| null nor undefined. | ||
|
|
||
| _Parameters_ | ||
|
|
||
| - _value_ `(T|undefined|null)`: The value to check. | ||
|
|
||
| _Returns_ | ||
|
|
||
| - (unknown type): Whether the value is undefined. | ||
|
|
||
| <a name="isObjectInterpolation" href="#isObjectInterpolation">#</a> **isObjectInterpolation** | ||
|
|
||
| There's no way to detect whether an `any` type is an `ObjectInterpolation` because `ObjectInterpolation`s | ||
| are merely objects where the keys are either (1) [CSS property names](https://github.com/emotion-js/emotion/blob/a72e6dc0f326b7d3d6067698d433018ee8c4cbf1/packages/serialize/types/index.d.ts#L10), | ||
| (2) [CSS Pseudo property names](https://github.com/emotion-js/emotion/blob/a72e6dc0f326b7d3d6067698d433018ee8c4cbf1/packages/serialize/types/index.d.ts#L18), | ||
| or (3) [anything else where the value for the key is another interpolation](https://github.com/emotion-js/emotion/blob/a72e6dc0f326b7d3d6067698d433018ee8c4cbf1/packages/serialize/types/index.d.ts#L19). | ||
| That means ObjectInterpolations can look all of the following ways: | ||
|
|
||
| ```js | ||
| const oi = { | ||
| color: 'black', | ||
| }; | ||
|
|
||
| const oi = { | ||
| ':disabled': css`color: black;`, | ||
| }; | ||
|
|
||
| const oi = { | ||
| '@media (prefers-color-scheme: light)': css({ | ||
| backgroundColor: 'black', | ||
| color: 'white', | ||
| }), | ||
| }; | ||
| ``` | ||
|
|
||
| Therefore, this function only accepts `TemplateStringsArray` and `Interpolation` for it's arguments rather than `any`, which | ||
| cannot be refined to an `ObjectInterpolation`. However, given any generic `Interpolation`, we _can_ tell whether it is an | ||
| `ObjectInterpolation` specifically by simply checking whether the value is "plain object" as defined by `lodash`'s `isPlainObject`. | ||
|
|
||
| Note: The links above reference a specific commit for **Emotion 10**. Emotion 11 is out but we're stuck on Emotion 10 until we can | ||
| re-write `create-styles` and upgrade all of `@wordpress/components` to use the new version of Emotion. | ||
|
|
||
| At that point we'll have to revisit this function anyway as it seems that the `ObjectInterpolation` type no longer | ||
| exists and that the concept has evolved. | ||
|
|
||
| _Parameters_ | ||
|
|
||
| - _value_ (unknown type): The value to check. | ||
|
|
||
| _Returns_ | ||
|
|
||
| - (unknown type): Whether the value is an ObjectInterpolation. | ||
|
|
||
|
|
||
| <!-- END TOKEN(Autogenerated API docs) --> | ||
|
|
||
| <br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| { | ||
| "name": "@wordpress/ui-utils", | ||
| "version": "0.0.1", | ||
| "description": "A collection of simple utilities for building user interfaces.", | ||
| "author": "The WordPress Contributors", | ||
| "license": "GPL-2.0-or-later", | ||
| "keywords": [ | ||
| "wordpress", | ||
| "gutenberg", | ||
| "utils" | ||
| ], | ||
| "homepage": "https://github.com/WordPress/gutenberg/tree/master/packages/ui-utils/README.md", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/WordPress/gutenberg.git", | ||
| "directory": "packages/ui-utils" | ||
| }, | ||
| "bugs": { | ||
| "url": "https://github.com/WordPress/gutenberg/issues" | ||
| }, | ||
| "files": [ | ||
| "build", | ||
| "build-module", | ||
| "build-types", | ||
| "src", | ||
| "*.md" | ||
| ], | ||
| "main": "build/index.js", | ||
| "module": "build-module/index.js", | ||
| "react-native": "src/index", | ||
| "types": "build-types", | ||
| "sideEffects": false, | ||
| "dependencies": { | ||
| "lodash": "^4.17.19" | ||
| }, | ||
| "publishConfig": { | ||
| "access": "public" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export { default as isDefined } from './is-defined'; | ||
| export { default as isObjectInterpolation } from './is-object-interpolation'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| /* eslint-disable jsdoc/valid-types */ | ||
| /** | ||
| * Checks whether a value is defined, asserting that it is neither | ||
| * null nor undefined. | ||
| * | ||
| * @template T | ||
| * @param {T | undefined | null} value The value to check. | ||
| * @return {value is T} Whether the value is undefined. | ||
| */ | ||
| const isDefined = ( value ) => value !== null && value !== undefined; | ||
|
There was a problem hiding this comment. This already exists in lodash There was a problem hiding this comment. You're right, but if we'd like to remove We could replace it with There was a problem hiding this comment. I'm not sure I agree with "getting rid of lodash by adding our own utils". I think getting rid of lodash by using native functions is definitely a good thing but other than that:
There was a problem hiding this comment. Sounds good to me. We'll replace |
||
|
|
||
| export default isDefined; | ||
| /* eslint-enable jsdoc/valid-types */ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| /* global TemplateStringsArray */ | ||
| /** | ||
| * External dependencies | ||
| */ | ||
| import isPlainObject from 'lodash/isPlainObject'; | ||
|
|
||
| /* eslint-disable jsdoc/valid-types */ | ||
| /** | ||
| * | ||
| * There's no way to detect whether an `any` type is an `ObjectInterpolation` because `ObjectInterpolation`s | ||
| * are merely objects where the keys are either (1) [CSS property names](https://github.com/emotion-js/emotion/blob/a72e6dc0f326b7d3d6067698d433018ee8c4cbf1/packages/serialize/types/index.d.ts#L10), | ||
| * (2) [CSS Pseudo property names](https://github.com/emotion-js/emotion/blob/a72e6dc0f326b7d3d6067698d433018ee8c4cbf1/packages/serialize/types/index.d.ts#L18), | ||
| * or (3) [anything else where the value for the key is another interpolation](https://github.com/emotion-js/emotion/blob/a72e6dc0f326b7d3d6067698d433018ee8c4cbf1/packages/serialize/types/index.d.ts#L19). | ||
| * That means ObjectInterpolations can look all of the following ways: | ||
| * | ||
| * ```js | ||
| * const oi = { | ||
| * color: 'black', | ||
| * }; | ||
| * | ||
| * const oi = { | ||
| * ':disabled': css`color: black;`, | ||
| * }; | ||
| * | ||
| * const oi = { | ||
| * '@media (prefers-color-scheme: light)': css({ | ||
| * backgroundColor: 'black', | ||
| * color: 'white', | ||
| * }), | ||
| * }; | ||
| * ``` | ||
| * | ||
| * Therefore, this function only accepts `TemplateStringsArray` and `Interpolation` for it's arguments rather than `any`, which | ||
| * cannot be refined to an `ObjectInterpolation`. However, given any generic `Interpolation`, we _can_ tell whether it is an | ||
| * `ObjectInterpolation` specifically by simply checking whether the value is "plain object" as defined by `lodash`'s `isPlainObject`. | ||
| * | ||
| * Note: The links above reference a specific commit for **Emotion 10**. Emotion 11 is out but we're stuck on Emotion 10 until we can | ||
| * re-write `create-styles` and upgrade all of `@wordpress/components` to use the new version of Emotion. | ||
| * | ||
| * At that point we'll have to revisit this function anyway as it seems that the `ObjectInterpolation` type no longer | ||
| * exists and that the concept has evolved. | ||
| * | ||
| * @param {TemplateStringsArray | import('create-emotion').Interpolation} value The value to check. | ||
| * @return {value is import('create-emotion').ObjectInterpolation} Whether the value is an ObjectInterpolation. | ||
| */ | ||
| const isObjectInterpolation = ( value ) => isPlainObject( value ); | ||
|
There was a problem hiding this comment. why not just use isPlainObject directly? There was a problem hiding this comment. The usefulness of this function comes primarily from the type defintions. It's impossible to refine the type any other way other than through type definitions which TypeScript will trust implicitly. This allows the newly added packages to be fully and completely typed. For example, given the following (real) code example: /**
* @param {TemplateStringsArray | import('create-emotion').Interpolation<undefined>} template
* @param {import('create-emotion').Interpolation<undefined>[]} args The styles to compile.
* @returns {ReturnType<compile>} The compiled styles.
*/
export function css(template, ...args) {
if (isObjectInterpolation(template)) {
return compile(responsive(template));
}
if (Array.isArray(template)) { // => TemplateStringsArray
for (let i = 0, len = template.length; i < len; i++) {
const n = template[i];
if (isObjectInterpolation(n)) {
template[i] = responsive(n);
}
}
return compile(template, ...args);
}
return compile(template, ...args);
}If you replace There was a problem hiding this comment. I don't think it's a valid reason for us at least to add a new package to be honest. Can't this be solved by an explicit type cast instead? There was a problem hiding this comment. Well, this package isn't complete as it stands, so it's justification can't live on just these two utilities. The issue linked in the PR description has a short list of utilities that would live in this package (and those are just the ones used by the FWIW I hadn't tried to type cast it but it does work, so we can remove this utility if you'd prefer to just use type casts (that's all this function does anyhow). Regardless, I'll open a separate PR with a different utility adding the same There was a problem hiding this comment. I think adding packages shouldn't be seen as something we should do easily especially in the context of WordPress, so I'd personally feel better if we start by doing inline utilities in the "components" package and discuss whether it deserves a separate package or not as the usage grows. There was a problem hiding this comment. What about utilities used by mulitple packages like There was a problem hiding this comment. What does There was a problem hiding this comment. /**
* Gets the displayName of a React Component or element.
*
* @param {string | import('react').ComponentType} tagName
*
* @return {string} The display name of the Component / tagName.
*/
export function getDisplayName(tagName) {
const displayName =
typeof tagName === 'string'
? tagName
: tagName.displayName || tagName.name || 'Component';
return displayName;
}There was a problem hiding this comment. So looking a g2. It seems to be only used once. That said it seems to be used to compute components display name? In fact we have something close to it in There was a problem hiding this comment. Ah, you're right, I thought |
||
| /* eslint-enable jsdoc/valid-types */ | ||
|
|
||
| export default isObjectInterpolation; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ui-utils as a name doesn't make sense for me. These utilities can be useful for any code. Why not
@wordpress/isThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this comment doesn't make sense if we decide to drop the package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for this PR and for this package is to provide a space for us to migrate the other utilities found in the G2 Component system:
https://github.com/ItsJonQ/g2/tree/master/packages/utils/src
I'm happy renaming it to
@wordpress/utilspackage if that were the case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to have a
@wordpress/utilsa long time ago and we removed it in favor of smaller packages with clear purpose. A@wordpress/iswould have been a good one here but IMO, lodash offers already all of these functions and since lodash is already used across our codebase, I'd just continue using it instead of replacing it with our custom lodash.