Skip to content

[pull] main from facebook:main #10

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

Merged
merged 6 commits into from
May 10, 2024
Merged

[pull] main from facebook:main #10

merged 6 commits into from
May 10, 2024

Conversation

pull[bot]
Copy link

@pull pull bot commented May 9, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

sebmarkbage and others added 2 commits May 9, 2024 12:23
This is the first step to experimenting with a new type of stack traces
behind the `enableOwnerStacks` flag - in DEV only.

The idea is to generate stacks that are more like if the JSX was a
direct call even though it's actually a lazy call. Not only can you see
which exact JSX call line number generated the erroring component but if
that's inside an abstraction function, which function called that
function and if it's a component, which component generated that
component. For this to make sense it really need to be the "owner" stack
rather than the parent stack like we do for other component stacks. On
one hand it has more precise information but on the other hand it also
loses context. For most types of problems the owner stack is the most
useful though since it tells you which component rendered this
component.

The problem with the platform in its current state is that there's two
ways to deal with stacks:

1) `new Error().stack` 
2) `console.createTask()`

The nice thing about `new Error().stack` is that we can extract the
frames and piece them together in whatever way we want. That is great
for constructing custom UIs like error dialogs. Unfortunately, we can't
take custom stacks and set them in the native UIs like Chrome DevTools.

The nice thing about `console.createTask()` is that the resulting stacks
are natively integrated into the Chrome DevTools in the console and the
breakpoint debugger. They also automatically follow source mapping and
ignoreLists. The downside is that there's no way to extract the async
stack outside the native UI itself so this information cannot be used
for custom UIs like errors dialogs. It also means we can't collect this
on the server and then pass it to the client for server components.

The solution here is that we use both techniques and collect both an
`Error` object and a `Task` object for every JSX call.

The main concern about this approach is the performance so that's the
main thing to test. It's certainly too slow for production but it might
also be too slow even for DEV.

This first PR doesn't actually use the stacks yet. It just collects them
as the first step. The next step is to start utilizing this information
in error printing etc.

For RSC we pass the stack along across over the wire. This can be
concatenated on the client following the owner path to create an owner
stack leading back into the server. We'll later use this information to
restore fake frames on the client for native integration. Since this
information quickly gets pretty heavy if we include all frames, we strip
out the top frame. We also strip out everything below the functions that
call into user space in the Flight runtime. To do this we need to figure
out the frames that represents calling out into user space. The
resulting stack is typically just the one frame inside the owner
component's JSX callsite. I also eagerly strip out things we expect to
be ignoreList:ed anyway - such as `node_modules` and Node.js internals.
Before this change, `useFormStatus` is only activated if a form is
submitted by an action function (either `<form action={actionFn}>` or
`<button formAction={actionFn}>`).

After this change, `useFormStatus` will also be activated if you call
`startTransition(actionFn)` inside a submit event handler that is
`preventDefault`-ed.

This is the last missing piece for implementing a custom `action` prop
that is progressively enhanced using `onSubmit` while maintaining the
same behavior as built-in form actions.

Here's the basic recipe for implementing a progressively-enhanced form
action. This would typically be implemented in your UI component
library, not regular application code:

```js
import {requestFormReset} from 'react-dom';

// To implement progressive enhancement, pass both a form action *and* a
// submit event handler. The action is used for submissions that happen
// before hydration, and the submit handler is used for submissions that
// happen after.
<form
  action={action}
  onSubmit={(event) => {
    // After hydration, we upgrade the form with additional client-
    // only behavior.
    event.preventDefault();

    // Manually dispatch the action.
    startTransition(async () => {
      // (Optional) Reset any uncontrolled inputs once the action is
      // complete, like built-in form actions do.
      requestFormReset(event.target);

      // ...Do extra action-y stuff in here, like setting a custom
      // optimistic state...

      // Call the user-provided action
      const formData = new FormData(event.target);
      await action(formData);
    });
  }}
/>
```
@pull pull bot added the ⤵️ pull label May 9, 2024
Instead of forcing an object to be outlined to be able to refer to it
later we can refer to it by the property path inside another parent
object.

E.g. this encodes such a reference as `'$123:props:children:foo:bar'`.

That way we don't have to preemptively outline object and we can dedupe
after the first time we've found it.

There's no cost on the client if it's not used because we're not storing
any additional information preemptively.

This works mainly because we only have simple JSON objects from the root
reference. Complex objects like Map, FormData etc. are stored as their
entries array in the look up and not the complex object. Other complex
objects like TypedArrays or imports don't have deeply nested objects in
them that can be referenced.

This solves the problem that we only dedupe after the third instance.
This dedupes at the second instance. It also solves the problem where
all nested objects inside deduped instances also are outlined.

The property paths can get pretty large. This is why a test on payload
size increased. We could potentially outline the reference itself at the
first dupe. That way we get a shorter ID to refer to in the third
instance.
Uses the same technique as in #28996 to encode references to already
emitted objects. This now means that Reply can support cyclic objects
too for parity.
…9010)

Stacked on #28997.

We can use the technique of referencing an object by its row + property
name path for temporary references - like we do for deduping. That way
we don't need to generate an ID for temporary references. Instead, they
can just be an opaque marker in the slot and it has the implicit ID of
the row + path.

Then we can stash all objects, even the ones that are actually available
to read on the server, as temporary references. Without adding anything
to the payload since the IDs are implicit. If the same object is
returned to the client, it can be referenced by reference instead of
serializing it back to the client. This also helps preserve object
identity.

We assume that the objects are immutable when they pass the boundary.

I'm not sure if this is worth it but with this mechanism, if you return
the `FormData` payload from a `useActionState` it doesn't have to be
serialized on the way back to the client. This is a common pattern for
having access to the last submission as "default value" to the form
fields. However you can still control it by replacing it with another
object if you want. In MPA mode, the temporary references are not
configured and so it needs to be serialized in that case. That's
required anyway for hydration purposes.

I'm not sure if people will actually use this in practice though or if
FormData will always be destructured into some other object like with a
library that turns it into typed data, and back. If so, the object
identity is lost.
@pull pull bot merged commit 6ef0dd4 into code:main May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants