Core Data: Replace spread arguments with non-spread variants #39477
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
|
Size Change: -41 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
adamziel
force-pushed
the
core-data/spread-argument-refactor
branch
from
f027416
to
d8ece5b
Compare
adamziel
approved these changes
Mar 16, 2022
adamziel
force-pushed
the
core-data/spread-argument-refactor
branch
2 times, most recently
from
d32cf82
to
180edac
Compare
When dynamically creating accessor and updator methods for core data entities we catch variable arguments in the methods and pass them through as spread arguments. While this offers a certain expandability should we update the underlying methods it also confuses the types of the interfaces since those underlying methods don't allow variable argument lists (or at least, they ignore anything past the known arguments). In this patch we're replacing those spread variants with direct named arguments to match the underlying inferface. This will present cleaner types and remove one bit of confusion.
adamziel
force-pushed
the
core-data/spread-argument-refactor
branch
from
180edac
to
ebaf9c9
Compare
dmsnell
added a commit
that referenced
this pull request
Mar 31, 2022
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
added a commit
that referenced
this pull request
Apr 1, 2022
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.
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.
Description
Part of #39211
When dynamically creating accessor and updator methods for core data entities
we catch variable arguments in the methods and pass them through as spread arguments.
While this offers a certain expandability should we update the underlying methods
it also confuses the types of the interfaces since those underlying methods don't
allow variable argument lists (or at least, they ignore anything past the known
arguments).
In this patch we're replacing those spread variants with direct named arguments
to match the underlying inferface. This will present cleaner types and remove
one bit of confusion.
Testing Instructions
Gutenberg has allowed passing more arguments to the entity accessor functions
than the underlying methods acknowledge or use. Test this patch by running a normal
array of operations that require fetching entity records. For example, test loading
block templates or FSE layouts.
Audit the code and verify that by removing the spread argument we aren't limiting
ourselves or ignoring ways these methods might be overwritten at runtime to accept
more arguments.