Flip the arguments of Blocks and make the query optional#18374
Conversation
facebook-github-bot
added
React Core Team
sebmarkbage
force-pushed
the
optionalquery
branch
from
38dc654 to
0c87b1a
Compare
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 38dc654:
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 2fa207c:
|
Details of bundled changes.Comparing: a56309f...2fa207c react
React: size: 🔺+0.5%, gzip: 🔺+0.1% Size changes (experimental) |
Details of bundled changes.Comparing: a56309f...2fa207c react
React: size: 0.0%, gzip: -0.0% Size changes (stable) |
| return [Symbol.for('react.server.block'), render]; | ||
| } | ||
| let curriedQuery = () => { | ||
| return query(...args); |
There was a problem hiding this comment.
Interestingly, if the query itself returns undefined, it will turn into null because of how JSON.stringify works on arrays. Fixing this would be kind of tricky but you're not really supposed to return undefined from a query anyway. We could warn for that same as we warn if you return undefined from render.
There was a problem hiding this comment.
We could warn for that same as we warn if you return undefined from render.
yeah let's warn in that case
| export function block<Args: Iterable<any>, Props, Data>( | ||
| query: BlockQueryFunction<Args, Data>, | ||
| render: BlockRenderFunction<Props, Data>, | ||
| query?: BlockQueryFunction<Args, Data>, |
There was a problem hiding this comment.
When declared for external users, the overloaded type is:
declare export function block<Args: Iterable<any>, Props, Data>(
render: BlockRenderFunction<Props, Data>,
query: BlockQueryFunction<Args, Data>,
): (...args: Args) => Block<Props>;
declare export function block<Props>(
render: BlockRenderFunction<Props, void>,
): () => Block<Props>;
|
I also renamed It is lower case because the significant function is the render function which is required. It gets to be the name of the component for debugging purposes. The load function is an additional extra. |
| } | ||
|
|
||
| let loadUser = block(Query, User); | ||
| let loadUser = block(User, load); |
There was a problem hiding this comment.
Unfortunate that the order gets reversed from the grammar. User + load = loadUser.
There was a problem hiding this comment.
That's just a naming convention that we could change, though. It's probably helpful to use different names for the result of block, since calling loadUser here doesn't directly call load.
| } | ||
| let loadUser = block(User); | ||
| let model = { | ||
| User: loadUser('Seb', 'Smith'), |
There was a problem hiding this comment.
the type signature for loadUser here would disallow args, though, right?
| } | ||
|
|
||
| let loadUser = block(Query, User); | ||
| let loadUser = block(User, load); |
There was a problem hiding this comment.
That's just a naming convention that we could change, though. It's probably helpful to use different names for the result of block, since calling loadUser here doesn't directly call load.
4 participants
The Query is an optional overload so I'll put that at the end instead. It also lines up with where the "data" argument goes into the render.
Originally I had some thought around how it would be better for types to infer the result of the first argument flowing into the second. Maybe as you're typing it out. But the current type systems and IDE wouldn't take advantage of this anyway so doesn't matter.
cc @devknoll