Skip to content

Conversation

sirreal
Copy link
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

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 on lines +7 to +8
"noImplicitAny": false,
"strictNullChecks": false
Copy link
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
Member Author

sirreal commented Mar 31, 2020

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

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
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
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
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
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
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