Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prettier Config: Add type-checking #21053

Merged
merged 2 commits into from Apr 1, 2020
Merged

Prettier Config: Add type-checking #21053

merged 2 commits into from Apr 1, 2020

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
@github-actions
Copy link

@github-actions 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

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

packages/prettier-config/lib/index.js Show resolved Hide resolved
packages/prettier-config/lib/index.js Show resolved Hide resolved
@gziolo
gziolo approved these changes Mar 21, 2020
@aduth
Copy link
Member Author

@aduth 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 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 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 😃

@sirreal sirreal force-pushed the try/output-typedefs branch from 901ce5d to 31ea1e7 Mar 25, 2020
@aduth aduth mentioned this pull request Mar 25, 2020
6 tasks done
@aduth
Copy link
Member Author

@aduth 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 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 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 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 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.

@sirreal sirreal force-pushed the try/output-typedefs branch from 449bb18 to 89a0540 Mar 26, 2020
@aduth aduth force-pushed the add/types-prettier-config branch from 783dac1 to 8dee3d0 Mar 31, 2020
@aduth aduth changed the base branch from try/output-typedefs to master Mar 31, 2020
@aduth
Copy link
Member Author

@aduth 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 Mar 31, 2020
@aduth aduth merged commit 8aa3ad4 into master Apr 1, 2020
3 checks passed
3 checks passed
@github-actions
build
Details
@github-actions
pull-request-automation
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
@aduth aduth deleted the add/types-prettier-config branch Apr 1, 2020
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 1, 2020
@aduth
Copy link
Member Author

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