Skip to content

Conversation

aduth
Copy link
Member

@aduth aduth commented Mar 20, 2020

Blocked by (current base): #18942
Related: #18838

This pull request seeks to add type-checking for the @wordpress/prettier-config package.

By virtue of #18942, it also implies that types will be output as part of package distribution, though this may or may not be a very useful package to reference.

The primary benefit of these changes is largely in verifying configuration keys and values.

It also benefits from the same sorts of thing I wrote about recently, wherein the addition of the type detail trivializes future configuration maintenance:

Prettier config

Finally, this pull request serves as a possible reference for the revised approach to opting in to TypeScript type-checking for a package as of #18942.

Testing Instructions:

Ensure type-checking passes:

npm run build:package-types

@aduth aduth added [Type] Enhancement A suggestion for improvement. [Tool] Prettier config /packages/prettier-config labels Mar 20, 2020
@aduth aduth requested review from gziolo and ntwb March 20, 2020 18:48
@aduth aduth added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Mar 20, 2020
@github-actions
Copy link

github-actions bot commented Mar 20, 2020

Size Change: +13.5 kB (1%)

Total Size: 883 kB

Filename Size Change
build/a11y/index.js 1.02 kB +19 B (1%)
build/annotations/index.js 3.45 kB +1 B
build/api-fetch/index.js 3.8 kB +410 B (10%) ⚠️
build/block-directory/index.js 6.02 kB +2 B (0%)
build/block-editor/index.js 102 kB +12 B (0%)
build/block-editor/style-rtl.css 11.2 kB +171 B (1%)
build/block-editor/style.css 11.2 kB +175 B (1%)
build/block-library/index.js 110 kB -148 B (0%)
build/blocks/index.js 57.5 kB -1 B
build/components/style-rtl.css 16.1 kB +296 B (1%)
build/components/style.css 16 kB +297 B (1%)
build/core-data/index.js 10.7 kB +1 B
build/data-controls/index.js 1.03 kB -1 B
build/edit-post/index.js 92.3 kB -1 B
build/edit-post/style-rtl.css 12 kB +3.67 kB (30%) 🚨
build/edit-post/style.css 12 kB +3.67 kB (30%) 🚨
build/edit-site/index.js 9.04 kB +11 B (0%)
build/edit-site/style-rtl.css 4.62 kB +1.2 kB (26%) 🚨
build/edit-site/style.css 4.61 kB +1.2 kB (26%) 🚨
build/edit-widgets/style-rtl.css 3.74 kB +1.17 kB (31%) 🚨
build/edit-widgets/style.css 3.74 kB +1.17 kB (31%) 🚨
build/editor/index.js 42.8 kB -1 B
build/editor/style-rtl.css 3.47 kB +84 B (2%)
build/editor/style.css 3.47 kB +81 B (2%)
build/keyboard-shortcuts/index.js 2.3 kB +1 B
build/media-utils/index.js 4.84 kB -2 B (0%)
build/server-side-render/index.js 2.54 kB -3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/autop/index.js 2.59 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-library/editor-rtl.css 7.21 kB 0 B
build/block-library/editor.css 7.21 kB 0 B
build/block-library/style-rtl.css 7.5 kB 0 B
build/block-library/style.css 7.51 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/components/index.js 195 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.37 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.4 kB 0 B
build/edit-navigation/style-rtl.css 95 B 0 B
build/edit-navigation/style.css 95 B 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 426 B 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.94 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/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/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 780 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/shortcode/index.js 1.7 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.6 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I assume the config file included is fine.

Noting that, It’s yet another thing to take into account when creating a package. Should we introduce package scaffolding tool? 😅

@aduth
Copy link
Member Author

aduth commented Mar 23, 2020

Noting that, It’s yet another thing to take into account when creating a package. Should we introduce package scaffolding tool? 😅

Yeah, part of this was to just to demonstrate (even for myself) the amount of effort which would be involved as of the changes resulting from #18942. I think a scaffolding tool could be useful. I was also considering whether it could be the sort of thing where a bot would comment on a pull request where a new package is being added, which could do a few things:

  • Include a checklist of tasks to complete (and automatically update them as checked once completed)
  • Reference relevant documentation
  • Ping the core development team (since it affects a public interface)

In theory, a bot could even do the commits itself.

@aduth
Copy link
Member Author

aduth commented Mar 24, 2020

Just to reiterate: This is currently blocked by #18942. My plan would be to wait for #18942 to be merged, then change the base of this pull request to master and merge.

@gziolo
Copy link
Member

gziolo commented Mar 24, 2020

It sounds like a plan 👍

I want to use this PR to add types for create block a scripts packages at some point in the near future 😃

@aduth
Copy link
Member Author

aduth commented Mar 25, 2020

@sirreal I see you did a force push recently that introduced quite a few unexpected changes and merge conflicts. Do you know what might have happened?

@sirreal
Copy link
Member

sirreal commented Mar 25, 2020

I force pushed to the base branch (#18942) after rebasing against master to fix conflicts.

@aduth
Copy link
Member Author

aduth commented Mar 25, 2020

Ah, I guess I didn't anticipate those sorts of changes in your branch would surface up so prominently in this pull request 🤔

No matter, I should be able to get it resolved without much trouble. Likely just needs a rebase against your newly-rebased branch.

@sirreal
Copy link
Member

sirreal commented Mar 25, 2020

I've pushed more changes to the base branch and appear to have introduced more conflicts. Sorry!

It's a sign that it's time to land #18942 😁

@aduth
Copy link
Member Author

aduth commented Mar 26, 2020

It's a sign that it's time to land #18942 😁

Yes 😄 , but also....

I've pushed more changes to the base branch and appear to have introduced more conflicts. Sorry!

It looks bad, but I doubt there are actually any conflicts to resolve.

@aduth aduth changed the base branch from try/output-typedefs to master March 31, 2020 19:35
@aduth
Copy link
Member Author

aduth commented Mar 31, 2020

Noting that I'm intentionally choosing not to include a CHANGELOG entry for this change because there isn't yet a published version of this package (i.e. it technically still qualified under the current "initial release" note).

@aduth aduth force-pushed the add/types-prettier-config branch from 8dee3d0 to 22803cb Compare March 31, 2020 20:45
@aduth aduth merged commit 8aa3ad4 into master Apr 1, 2020
@aduth aduth deleted the add/types-prettier-config branch April 1, 2020 12:26
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 1, 2020
@aduth
Copy link
Member Author

aduth commented Apr 3, 2020

I neglected to include necessary changes here. Don't use this pull request as reference 😬

See #21381

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Blocked Used to indicate that a current effort isn't able to move forward [Tool] Prettier config /packages/prettier-config [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants