Skip to content

Element: Add types#21248

Merged
sirreal merged 14 commits into
masterfrom
try/type-wp-element
Apr 4, 2020
Merged

Element: Add types#21248
sirreal merged 14 commits into
masterfrom
try/type-wp-element

Conversation

@sirreal
Copy link
Copy Markdown
Member

@sirreal sirreal commented Mar 29, 2020

Description

Add types/typechecking to @wordpress/element. Many other packages that would be extremely valuable to type, namely @wordpress/data and @wordpress/components, depend on this package. Typing it unblocks a lot of work on other package typings.

  • Improve internal JSDoc/TypeScript typings.
  • Add published types.

Add unknown and never TypeScript types to allowed JSDoc types.

Includes squash-merged #20669, necessary for some required types.

Part of #18838

How has this been tested?

npm run build:package-types passes.

Screenshots

Types of changes

New feature: Add types

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@sirreal sirreal self-assigned this Mar 29, 2020
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 29, 2020

Size Change: -3 B (0%)

Total Size: 889 kB

Filename Size Change
build/element/index.js 4.44 kB -3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.4 kB 0 B
build/api-fetch/index.js 3.79 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 102 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.22 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.53 kB 0 B
build/block-library/style.css 7.54 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.05 kB 0 B
build/edit-navigation/index.js 2.71 kB 0 B
build/edit-navigation/style-rtl.css 239 B 0 B
build/edit-navigation/style.css 241 B 0 B
build/edit-post/index.js 92.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.1 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.18 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 426 B 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Comment thread packages/element/tsconfig.json Outdated
Comment on lines 7 to 8
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removing these lines:

  • noImplicitAny -> 51 errors
  • strictNullChecks -> 7 errors
  • both -> 60 errors

@sirreal sirreal force-pushed the try/type-wp-element branch from ca897c8 to c3549e2 Compare March 29, 2020 22:25
@sirreal sirreal marked this pull request as ready for review March 29, 2020 22:34
@sirreal sirreal added [Package] Element /packages/element [Type] Code Quality Issues or PRs that relate to code quality labels Mar 29, 2020
@sirreal sirreal requested a review from dsifford March 30, 2020 09:28
@sirreal sirreal force-pushed the try/type-wp-element branch from 7088eae to d3fe07e Compare March 31, 2020 19:00
@sirreal
Copy link
Copy Markdown
Member Author

sirreal commented Mar 31, 2020

Rebased and removed commits from #20669 which has been merged.

Comment thread packages/element/README.md Outdated
Comment thread packages/element/src/create-interpolate-element.js
Comment thread packages/element/src/serialize.js Outdated
Comment thread packages/eslint-plugin/configs/jsdoc.js
Comment thread packages/element/src/serialize.js
Comment thread packages/element/src/serialize.js
sirreal and others added 3 commits April 3, 2020 22:28
Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>
Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>
@sirreal sirreal force-pushed the try/type-wp-element branch from 06b1bf9 to 8bed5cf Compare April 3, 2020 20:45
@sirreal
Copy link
Copy Markdown
Member Author

sirreal commented Apr 3, 2020

Rebased to fix conflict. Would it be possible to move forward with this? Many packages depend on element so it's a a bottleneck.

@sirreal sirreal added the Needs Technical Feedback Needs testing from a developer perspective. label Apr 3, 2020
Copy link
Copy Markdown
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

👍

@sirreal sirreal merged commit fe0d10c into master Apr 4, 2020
@sirreal sirreal deleted the try/type-wp-element branch April 4, 2020 20:57
@github-actions github-actions Bot added this to the Gutenberg 7.9 milestone Apr 4, 2020
This was referenced Apr 8, 2020
Comment on lines -16 to +20
* @return {WPComponent} Dangerously-rendering component.
* @return {JSX.Element} Dangerously-rendering component.
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.

I wondered how this worked.

Turns out it doesn't:

[0] /home/travis/build/WordPress/gutenberg/packages/element/src/raw-html.js
[0]   20:0  warning  The type 'JSX' is undefined  jsdoc/no-undefined-types

https://travis-ci.com/github/WordPress/gutenberg/jobs/314225929

I thought these rules were upgraded to be errors in #20427. I think this one might have been missed? (cc @sainthkh)

I know we have a handful of temporary exceptions for custom types, but pretty sure those are hard-coded to be exceptions and could still exist even if the rule were upgraded from warning to error.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is this a concern with restoring the element types? #21781

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It happened because I missed this one rule:

'jsdoc/no-undefined-types': [
'warn',

I opened an issue that fixes it: #21942.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Technical Feedback Needs testing from a developer perspective. [Package] Element /packages/element [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants