Types: Hide element, primitives, icons declarations #21613
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove types from pacakge.json to hide the included types. This was reported to cause conflicts with the current 3rd party typings published on DefinitelyTyped (DT). The cause has been identified as: - DT exports all React typings, which other DT typings depend on. - Included typings use type aliases (Element -> WPElement) which may cause additional conflicts. See https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/wordpress__element/index.d.ts See https://unpkg.com/browse/@wordpress/element@2.13.0/build-types/react.d.ts Reported via core-js slack: https://wordpress.slack.com/archives/C5UNMSU4R/p1586955177257000
sirreal
requested review from
ajitbohra,
jorgefilipecosta and
youknowriad
as code owners
sirreal
changed the title
|
Can we include a brief CHANGELOG note? For those looking to upgrade again after the latest round of published updates. |
aduth
approved these changes
Apr 15, 2020
|
Added a changelog entry. |
|
We'll end up having to back out more types if we go with this approach 😞 I believe this will cascade into |
|
Size Change: 0 B Total Size: 839 kB ℹ️ View Unchanged
|
sirreal
changed the title
sirreal
added
[Package] Icons
gziolo
pushed a commit
that referenced
this pull request
Apr 15, 2020
Unpublish @wordpress/element, @wordpress/primitives, and @wordpress/icons types Remove types from pacakge.json to hide the included types. This was reported to cause conflicts with the current 3rd party typings published on DefinitelyTyped (DT). The cause has been identified as: - DT exports all React typings, which other DT typings depend on. - Included typings use type aliases (Element -> WPElement) which may cause additional conflicts. See https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/wordpress__element/index.d.ts See https://unpkg.com/browse/@wordpress/element@2.13.0/build-types/react.d.ts Reported via core-js slack: https://wordpress.slack.com/archives/C5UNMSU4R/p1586955177257000 * Add CHANGELOG * Remove primitives, icons types
sirreal
added a commit
that referenced
this pull request
Apr 15, 2020
75 tasks
|
An unfortunate side-effect of this is that we can't even use these package types internally. I sense this is what you'd noticed in needing to remove Looking at our implementation prior to #18942, I was wondering if there's something we could be doing similar to what we had with this now-removed configuration: I'm running into errors at #21467 due to lack of availability of diff --git a/packages/block-editor/tsconfig.json b/packages/block-editor/tsconfig.json
index aa711da37..57fcd356d 100644
--- a/packages/block-editor/tsconfig.json
+++ b/packages/block-editor/tsconfig.json
@@ -2,7 +2,14 @@
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"rootDir": "src",
- "declarationDir": "build-types"
+ "declarationDir": "build-types",
+ "baseUrl": "../..",
+ "paths": {
+ "@wordpress/*": [
+ "packages/*/index.js",
+ "packages/*/src/index.js"
+ ]
+ },
},
"references": [
{ "path": "../element" },Do you think that's a sensible temporary solution to be able to use these types internally? For now, I'll probably back out my type-checking changes in #21467. |
This was referenced Apr 16, 2020
|
I hope to get back to this with some proposals in the next few days.
Thanks for the patience. |
This was referenced Apr 21, 2020
|
I've opened #21767 to discuss details and how to proceed. |
1 task
sirreal
added a commit
that referenced
this pull request
Apr 22, 2020
[#21613](#21613) "hid" TypeScript emitted type declarations for `@wordpress/element` when it was found to conflict with 3rd party type declarations on DefinitelyTyped. Disabling `@wordpress/element` cascaded to require disabling type checking for `@wordpress/icons` and `@wordpress/primitives`, two dependent packages. The lint-staged flow is slightly different from the full `build:package-types` script in that it attempts to do a minimal build in the interest of speed. This flow checks for the presence of a `tsconfig.json` file in the root of the package with changes and runs its package build. While removing the affected packages from the main tsconfig.json did remove them from the primary build, changes to primitives and icons packages will fail because they cannot find the `@wordpress/element` types as was intended in #21613. By renaming the package tsconfig.json files, they are excluded from the lint-staged typechecking as well.
sirreal
added a commit
that referenced
this pull request
Apr 22, 2020
[#21613](#21613) "hid" TypeScript emitted type declarations for `@wordpress/element` when it was found to conflict with 3rd party type declarations on DefinitelyTyped. Disabling `@wordpress/element` cascaded to require disabling type checking for `@wordpress/icons` and `@wordpress/primitives`, two dependent packages. The lint-staged flow is slightly different from the full `build:package-types` script in that it attempts to do a minimal build in the interest of speed. This flow checks for the presence of a `tsconfig.json` file in the root of the package with changes and runs its package build. While removing the affected packages from the main tsconfig.json did remove them from the primary build, changes to primitives and icons packages will fail because they cannot find the `@wordpress/element` types as was intended in #21613. By renaming the package tsconfig.json files, they are excluded from the lint-staged typechecking as well.
sirreal
added a commit
that referenced
this pull request
Apr 28, 2020
* Restore element, icons, primitives types * Partially reverts #21613 (c2684b2) * Reverts #21784 (74fcd4c) Some `@wordpress/*` emit TypeScript type declarations and include them with the published packages. Shortly after publishing, it was discovered the `@wordpress/element` conflicted with the DefinitelyTyped 3rd party type declarations of several packages and a patch release was published removing the published types from `@wordpress/element`, `@wordpress/primitives`, and `@wordpress/icons` (#21613, #21784). The DefinitelyTyped type declarations have been updated to remove conflicts with the included `@wordpress/element` types. The types can be published again. See #21767 for more details.
aduth
added a commit
that referenced
this pull request
Apr 30, 2020
* Rename icons, primitives tsconfig to _tsconfig [#21613](#21613) "hid" TypeScript emitted type declarations for `@wordpress/element` when it was found to conflict with 3rd party type declarations on DefinitelyTyped. Disabling `@wordpress/element` cascaded to require disabling type checking for `@wordpress/icons` and `@wordpress/primitives`, two dependent packages. The lint-staged flow is slightly different from the full `build:package-types` script in that it attempts to do a minimal build in the interest of speed. This flow checks for the presence of a `tsconfig.json` file in the root of the package with changes and runs its package build. While removing the affected packages from the main tsconfig.json did remove them from the primary build, changes to primitives and icons packages will fail because they cannot find the `@wordpress/element` types as was intended in #21613. By renaming the package tsconfig.json files, they are excluded from the lint-staged typechecking as well. * Update arrow-left, cog, and help icons * Add "bar chart" icon * Add "home" icon * Add "megaphone" icon * Add "people" icon * Add "plugin" icon * Add "support" icon * Add "tool" icon * icon definitions * Update "support" icon dimensions and rename to "lifesaver" * Update star icons Replaces old Dashicons stars with g2 equivalents * Update star icon dimensions * star-half viewbox * E2E Tests: Update Plugins snapshots * Camelcase Path attributes * Update "tool" icon * Update icons package changelog * Update packages/icons/package.json Co-authored-by: Andrew Duthie <andrew@andrewduthie.com> * Update packages/icons/CHANGELOG.md Co-authored-by: Andrew Duthie <andrew@andrewduthie.com> Co-authored-by: Jon Surrell <jon.surrell@automattic.com> Co-authored-by: Andrew Duthie <andrew@andrewduthie.com>
This was referenced Mar 18, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove types from pacakge.json to hide the included types. This was
reported to cause conflicts with the current 3rd party typings published
on DefinitelyTyped (DT).
The cause has been identified as:
cause additional conflicts.
The
@wordpress/primitivesand@wordpress/iconspackages depend on@wordpress/element, so their types have been disabled as well.See https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/wordpress__element/index.d.ts
See https://unpkg.com/browse/@wordpress/element@2.13.0/build-types/react.d.ts
Reported by @dsifford via core-js slack:
https://wordpress.slack.com/archives/C5UNMSU4R/p1586955177257000