Core Data: Fixes necessary for converting to TypeScript #39211
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
This was referenced Mar 4, 2022
gziolo
added
[Type] Technical Prototype
dmsnell
added a commit
that referenced
this pull request
Mar 4, 2022
Part of #39211 Fixes a few small issues with the JSDoc types in @wordpress/core-data. - `undefined` was the wrong return type for the given files. removing that type to remove the error and leaving them untyped. - acknowledge that the `undoIgnore` parameter is optional for `editEntityRecord` - return type of `__experimentalBatch` is a function that returns a `Promise`, not a `Promise` Co-authored-by: Adam Zielinski <[email protected]>
dmsnell
added a commit
that referenced
this pull request
Mar 4, 2022
Part of #39211 Fixes a few small issues with the JSDoc types in @wordpress/core-data. - queue of actions in `batch` was untyped, added type holding vague structure of type - `await`'ed `Promise` called `resolve()` but wasn't typed as `Promise<void>`. this may not be possible in JSDoc whereas it is in `.ts` with the type parameter. I've made it `resolve(undefined)` to get around this. - inner loop of batched actions had ambiguous types (list of things or tuple?) and I've extracted the inline assignments to variables in order to eliminate the ambiguity - `ObservedSet` used spread arguments for `Set`'s 1-arity functions. I've replaced `...args` with `value` to match the API of `Set`
dmsnell
force-pushed
the
types/core-data-fixes
branch
2 times, most recently
from
f94896c to
93d8751
Compare
8 tasks
This was referenced Mar 15, 2022
dmsnell
force-pushed
the
types/core-data-fixes
branch
from
93d8751 to
73c4f18
Compare
dmsnell
added a commit
that referenced
this pull request
Mar 17, 2022
Part of #39211 Previously it was possible that we fail to register a batch size from the server and we leave the batch size null. In this patch we fallback to the Core-defined default of 25.
dmsnell
added a commit
that referenced
this pull request
Mar 17, 2022
Part of #39211 Fixes minor type-related issues that are mostly problems with JSDoc type signatures. In a couple of places a safe-defaulting fallback has been added where the existing JS code assumes the presence of nullable data.
This was referenced Mar 17, 2022
dmsnell
added a commit
that referenced
this pull request
Mar 17, 2022
Part of #39211 Previously we have been calling `createContext()` without any arguments, but there's a funny note in the official React type for that function. > // If you thought this should be optional, see > // DefinitelyTyped/DefinitelyTyped#24509 (comment) Although not passing an argument is practically the same as passing `undefined` as the argument we have a type error that TypeScript doesn't like while we're relying on it to parse JS files with the JSDoc typings. In this patch we're just adding the explicit `undefined` which should have no behavioral change on the output but removes the type issue.
adamziel
pushed a commit
that referenced
this pull request
Mar 17, 2022
Part of #39211 Fixes minor type-related issues that are mostly problems with JSDoc type signatures. In a couple of places a safe-defaulting fallback has been added where the existing JS code assumes the presence of nullable data.
adamziel
pushed a commit
that referenced
this pull request
Mar 17, 2022
…#39526) Part of #39211 Previously we have been calling `createContext()` without any arguments, but there's a funny note in the official React type for that function. > // If you thought this should be optional, see > // DefinitelyTyped/DefinitelyTyped#24509 (comment) Although not passing an argument is practically the same as passing `undefined` as the argument we have a type error that TypeScript doesn't like while we're relying on it to parse JS files with the JSDoc typings. In this patch we're just adding the explicit `undefined` which should have no behavioral change on the output but removes the type issue.
adamziel
force-pushed
the
types/core-data-fixes
branch
from
4254297 to
7215537
Compare
|
This is closer and closer @dmsnell, great work! I rebased on top of latest |
dmsnell
added a commit
that referenced
this pull request
Mar 22, 2022
Part of #39211 Previously it was possible that we fail to register a batch size from the server and we leave the batch size null. In this patch we fallback to the Core-defined default of 25.
dmsnell
added a commit
that referenced
this pull request
Mar 22, 2022
Part of #39211 The `Function` type in `onSubKey` caused an issue when enabling TypeScript for the `core-data` module (when analyzing JS files) because it's not a callable interface. In this patch we're creating a transition type `AnyFunction` and using that as the type in `onSubKey` to remove the type error. After enabling TypeScript for the module we'll be able to provide better types on the function directly, but for now this prevents raising an error. We want to do great things with the `core-data` type system but all of these little nuisances get in the way when we're trying to study those things in isolation. This is a preparatory change to eliminate some of the noise in the existing types. As a type-only change this will have no impact on the built code. Please audit the types and the introduction of the `types.ts` where we import an actual TypeScript type into JSDoc through `@typedef`. Note that this change doesn't "fully-type" `onSubKey` in the sense that it provides full inference for the passed-through reducer. The goal is merely to avoid creating type errors when activating TypeScript across the module.
dmsnell
added a commit
that referenced
this pull request
Mar 22, 2022
Part of #39211 In this patch we're expanding the type signature of `useQuerySelect` and fixing a minor type-related issue that was formerly there. We want to do great things with the `core-data` type system but all of these little nuisances get in the way when we're trying to study those things in isolation. This is a preparatory change to eliminate some of the noise in the existing types. As a type-only change this will have no impact on the built code. Please audit the types and the use of the wrapper `EnrichedSelectors`.
dmsnell
added a commit
that referenced
this pull request
Mar 22, 2022
Part of #39211 In this patch we're adding some types to the undo state tracked in the core data store. Additionally we're intializing it in one atomic operation whereas previously that took two partial assignments. Futher, a JSDoc comment has been moved from its previously erroneous location and back to the function it describes. This error had been overlooked in a previous change to the module. The purpose is to remove type issues when toggling on TypeScript across the `core-data` package. This is almost entirely a type-level change but the initialization of `UNDO_INITIAL_STATE` has been collapsed into a single assignment. Verify that the editor undo/redo functionality works as expected. Given that the one code change impacts the initialization it should be quickly obvious if there's a problem or regression with undo/redo.
jostnes
pushed a commit
to jostnes/gutenberg
that referenced
this pull request
Mar 23, 2022
Part of WordPress#39211 Fixes minor type-related issues that are mostly problems with JSDoc type signatures. In a couple of places a safe-defaulting fallback has been added where the existing JS code assumes the presence of nullable data.
jostnes
pushed a commit
to jostnes/gutenberg
that referenced
this pull request
Mar 23, 2022
…WordPress#39526) Part of WordPress#39211 Previously we have been calling `createContext()` without any arguments, but there's a funny note in the official React type for that function. > // If you thought this should be optional, see > // DefinitelyTyped/DefinitelyTyped#24509 (comment) Although not passing an argument is practically the same as passing `undefined` as the argument we have a type error that TypeScript doesn't like while we're relying on it to parse JS files with the JSDoc typings. In this patch we're just adding the explicit `undefined` which should have no behavioral change on the output but removes the type issue.
dmsnell
added a commit
that referenced
this pull request
Mar 28, 2022
Part of #39211 In this patch we're adding some types to the undo state tracked in the core data store. Additionally we're intializing it in one atomic operation whereas previously that took two partial assignments. Futher, a JSDoc comment has been moved from its previously erroneous location and back to the function it describes. This error had been overlooked in a previous change to the module. The purpose is to remove type issues when toggling on TypeScript across the `core-data` package. This is almost entirely a type-level change but the initialization of `UNDO_INITIAL_STATE` has been collapsed into a single assignment. Verify that the editor undo/redo functionality works as expected. Given that the one code change impacts the initialization it should be quickly obvious if there's a problem or regression with undo/redo.
dmsnell
added a commit
that referenced
this pull request
Mar 31, 2022
Part of #39211 In this patch we're adding some types to the undo state tracked in the core data store. Additionally we're intializing it in one atomic operation whereas previously that took two partial assignments. Futher, a JSDoc comment has been moved from its previously erroneous location and back to the function it describes. This error had been overlooked in a previous change to the module. The purpose is to remove type issues when toggling on TypeScript across the `core-data` package. This is almost entirely a type-level change but the initialization of `UNDO_INITIAL_STATE` has been collapsed into a single assignment. Verify that the editor undo/redo functionality works as expected. Given that the one code change impacts the initialization it should be quickly obvious if there's a problem or regression with undo/redo.
dmsnell
added a commit
that referenced
this pull request
Mar 31, 2022
Part of #39211 The `Function` type in `onSubKey` caused an issue when enabling TypeScript for the `core-data` module (when analyzing JS files) because it's not a callable interface. In this patch we're creating a transition type `AnyFunction` and using that as the type in `onSubKey` to remove the type error. After enabling TypeScript for the module we'll be able to provide better types on the function directly, but for now this prevents raising an error. We want to do great things with the `core-data` type system but all of these little nuisances get in the way when we're trying to study those things in isolation. This is a preparatory change to eliminate some of the noise in the existing types. As a type-only change this will have no impact on the built code. Please audit the types and the introduction of the `types.ts` where we import an actual TypeScript type into JSDoc through `@typedef`. Note that this change doesn't "fully-type" `onSubKey` in the sense that it provides full inference for the passed-through reducer. The goal is merely to avoid creating type errors when activating TypeScript across the module.
dmsnell
added a commit
that referenced
this pull request
Mar 31, 2022
Part of #39211 In this patch we're expanding the type signature of `useQuerySelect` and fixing a minor type-related issue that was formerly there. We want to do great things with the `core-data` type system but all of these little nuisances get in the way when we're trying to study those things in isolation. This is a preparatory change to eliminate some of the noise in the existing types. As a type-only change this will have no impact on the built code. Please audit the types and the use of the wrapper `EnrichedSelectors`.
In this commit we're cleaning up type issues in the core-data package that prevent us from telling TypeScript to run on the package and all of its existing code, even the JS files. After these changes we should be able to do so and start converting more modules to TypeScript with less friction. This patch follows a series of other smaller updates: - #39212 - #39214 - #39225 - #39476 - #39477 - #39479 - #39480 - #39525 - #39526 - #39655 - #39656 - #39659 It was built in order to support ongoing work to add types to the `getEntityRecord` family of functions in #39025.
dmsnell
force-pushed
the
types/core-data-fixes
branch
from
b0d121e to
68306ab
Compare
dmsnell
removed
the
[Type] Technical Prototype
dmsnell
requested review from
nerrad,
ajitbohra,
mkaz,
ryanwelcher and
juanmaguitar
as code owners
|
Size Change: 0 B Total Size: 1.22 MB ℹ️ View Unchanged
|
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.
In this commit we're cleaning up type issues in the core-data package
that prevent us from telling TypeScript to run on the package and all
of its existing code, even the JS files.
After these changes we should be able to do so and start converting
more modules to TypeScript with less friction.
This patch follows a series of other smaller updates:
typesdirectory toentity-types#39225undefinedinitial value tocreateContext#39526It was built in order to support ongoing work to add types to the
getEntityRecordfamily of functions in #39025.Testing
As a type-only change this shouldn't impact the generated bundles.
Please review the types and the fixes.
Note that the purpose isn't to provide helpful and long-term type
signatures but enable TypeScript support across the entire package.