syncing with main #1
Merged
Conversation
This file contains hidden or 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
Builds on top of facebook#26661 This lets you pass FormData objects through the Flight Reply serialization. It does that by prefixing each entry with the ID of the reference and then the decoding side creates a new FormData object containing only those fields (without the prefix). Ideally this should be more generic. E.g. you should be able to pass Blobs, Streams and Typed Arrays by reference inside plain objects too. You should also be able to send Blobs and FormData in the regular Flight serialization too so that they can go both directions. They should be symmetrical. We'll get around to adding more of those features in the Flight protocol as we go. --------- Co-authored-by: Sophie Alpert <[email protected]>
…docked devtools window (facebook#26665) bugfix for facebook#26492 This bug would cause users unable to use the devtools (component tree empty). The else-if logic is broken when user switch to undocked devtools mode (separate window) because `sender.tab` would exist in that case. <img width="314" alt="image" src="https://user-images.githubusercontent.com/1001890/232930094-05a74445-9189-4d50-baf1-a0360b29ef7e.png"> Tested on Chrome with a local build
This is kind of annoying because Date implements toJSON so JSON.stringify turns it into a string before calling our replacer function.
facebook#26660) Some minor changes, observed while working on 24.7.5 release: - Updated numeration of text instructions - `reactjs.org` -> `react.dev` - Fixed using `npm view` command for node 16+, `publish-release` script currently fails if used with node 16+
… feature flags (facebook#26635) ## Summary Removing `enableNamedHooksFeature`, `enableProfilerChangedHookIndices`, `enableProfilerComponentTree` feature flags, they are the same for all configurations.
) I accidentally made a behavior change in the refactor. It turns out that when switching off `checked` to an uncontrolled component, we used to revert to the concept of "initialChecked" which used to be stored on state. When there's a diff to this computed prop and the value of props.checked is null, then we end up in a case where it sets `checked` to `initialChecked`: https://github.com/facebook/react/blob/5cbe6258bc436b1683080a6d978c27849f1d9a22/packages/react-dom-bindings/src/client/ReactDOMInput.js#L69 Since we never changed `initialChecked` and it's not relevant if non-null `checked` changes value, the only way this "change" could trigger was if we move from having `checked` to having null. This wasn't really consistent with how `value` works, where we instead leave the current value in place regardless. So this is a "bug fix" that changes `checked` to be consistent with `value` and just leave the current value in place. This case should already have a warning in it regardless since it's going from controlled to uncontrolled. Related to that, there was also another issue observed in facebook#26596 (comment) and facebook#26588 We need to atomically apply mutations on radio buttons. I fixed this by setting the name to empty before doing mutations to value/checked/type in updateInput, and then set the name to whatever it should be. Setting the name is what ends up atomically applying the changes. --------- Co-authored-by: Sophie Alpert <[email protected]>
This creates 2 special branches. If these special branches exist, we'll commit build artifacts from these branches, main otherwise.
Implements initial (client-only) support for async actions behind a flag. This is an experimental feature and the design isn't completely finalized but we're getting closer. It will be layered alongside other features we're working on, so it may not feel complete when considered in isolation. The basic description is you can pass an async function to `startTransition` and all the transition updates that are scheduled inside that async function will be grouped together. The `isPending` flag will be set to true immediately, and only set back to false once the async action has completed (as well as all the updates that it triggers). The ideal behavior would be that all updates spawned by the async action are automatically inferred and grouped together; however, doing this properly requires the upcoming (stage 2) Async Context API, which is not yet implemented by browsers. In the meantime, we will fake this by grouping together all transition updates that occur until the async function has terminated. This can lead to overgrouping between unrelated actions, which is not wrong per se, just not ideal. If the `useTransition` hook is removed from the UI before an async action has completed — for example, if the user navigates to a new page — subsequent transitions will no longer be grouped with together with that action. Another consequence of the lack of Async Context is that if you call `setState` inside an action but after an `await`, it must be wrapped in `startTransition` in order to be grouped properly. If we didn't require this, then there would be no way to distinguish action updates from urgent updates caused by user input, too. This is an unfortunate footgun but we can likely detect the most common mistakes using a lint rule. Once Async Context lands in browsers, we can start warning in dev if we detect an update that hasn't been wrapped in `startTransition`. Then, longer term, once the feature is ubiquitous, we can rely on it for real and allow you to call `setState` without the additional wrapper. Things that are _not_ yet implemented in this PR, but will be added as follow ups: - Support for non-hook form of `startTransition` - Canceling the async action scope if the `useTransition` hook is deleted from the UI - Anything related to server actions
This lets you pass a function to `<form action={...}>` or `<button
formAction={...}>` or `<input type="submit formAction={...}>`. This will
behave basically like a `javascript:` URL except not quite implemented
that way. This is a convenience for the `onSubmit={e => {
e.preventDefault(); const fromData = new FormData(e.target); ... }`
pattern.
You can still implement a custom `onSubmit` handler and if it calls
`preventDefault`, it won't invoke the action, just like it would if you
used a full page form navigation or javascript urls. It behaves just
like a navigation and we might implement it with the Navigation API in
the future.
Currently this is just a synchronous function but in a follow up this
will accept async functions, handle pending states and handle errors.
This is implemented by setting `javascript:` URLs, but these only exist
to trigger an error message if something goes wrong instead of
navigating away. Like if you called `stopPropagation` to prevent React
from handling it or if you called `form.submit()` instead of
`form.requestSubmit()` which by-passes the `submit` event. If CSP is
used to ban `javascript:` urls, those will trigger errors when these
URLs are invoked which would be a different error message but it's still
there to notify the user that something went wrong in the plumbing.
Next up is improving the SSR state with action replaying and progressive
enhancement.
- substr is Annex B - substring silently flips its arguments if they're in the "wrong order", which is confusing - slice is better than sliced bread (no pun intended) and also it works the same way on Arrays so there's less to remember --- > I'd be down to just lint and enforce a single form just for the potential compression savings by using a repeated string. _Originally posted by @sebmarkbage in facebook#26663 (comment)
There is so much old stuff in these files. I am weeping.
Full list of changes: * Use .slice() for all substring-ing ([sophiebits](https://github.com/sophiebits) in [facebook#26677](facebook#26677)) * cleanup[devtools]: remove named hooks & profiler changed hook indices feature flags ([hoxyq](https://github.com/hoxyq) in [facebook#26635](facebook#26635)) * chore[devtools/release-scripts]: update messages / fixed npm view com… ([hoxyq](https://github.com/hoxyq) in [facebook#26660](facebook#26660)) * (patch)[DevTools] bug fix: backend injection logic not working for undocked devtools window ([mondaychen](https://github.com/mondaychen) in [facebook#26665](facebook#26665)) * use backend manager to support multiple backends in extension ([mondaychen](https://github.com/mondaychen) in [facebook#26615](facebook#26615))
This puts the change introduced by facebook#26611 behind a flag until Meta is able to roll it out. Disabling the flag reverts back to the old behavior, where retries are throttled if there's still data remaining in the tree, but not if all the data has finished loading. The new behavior is still enabled in the public builds.
…6570) In anticipation of making Fiber use the document global for dispatching Float methods that arrive from Flight I needed to update some tests that commonly recreated the JSDOM instance after importing react. This change updates a few tests to only create JSDOM once per test, before importing react-dom/client. Additionally the current act implementation for server streaming did not adequately model streaming semantics so I rewrite the act implementation in a way that better mirrors how a browser would parse incoming HTML. The new act implementation does the following 1. the first time it processes meaningful streamed content it figures out whether it is rendering into the existing document container or if it needs to reset the document. this is based on whether the streamed content contains tags `<html>` or `<body>` etc... 2. Once the streaming container is set it will typically continue to stream into that container for future calls to act. The exception is if the streaming container is the `<head>` in which case it will switch to streaming into the body once it receives a `<body>` tag. This means for tests that render something like a `<div>...</div>` it will naturally stream into the default `<div id="container">...` and for tests that render a full document the HTML will parse like a real browser would (with some very minor edge case differences) I also refactored the way we move nodes from buffered content into the document and execute any scripts we find. Previously we were using window.eval and I switched this to just setting the external script content as script text. Additionally the nonce logic is reworked to be a bit simpler.
…acebook#26557) Stacked on facebook#26570 Previously we restricted Float methods to only being callable while rendering. This allowed us to make associations between calls and their position in the DOM tree, for instance hoisting preinitialized styles into a ShadowRoot or an iframe Document. When considering how we are going to support Flight support in Float however it became clear that this restriction would lead to compromises on the implementation because the Flight client does not execute within the context of a client render. We want to be able to disaptch Float directives coming from Flight as soon as possible and this requires being able to call them outside of render. this patch modifies Float so that its methods are callable anywhere. The main consequence of this change is Float will always use the Document the renderer script is running within as the HoistableRoot. This means if you preinit as style inside a component render targeting a ShadowRoot the style will load in the ownerDocument not the ShadowRoot. Practially speaking it means that preinit is not useful inside ShadowRoots and iframes. This tradeoff was deemed acceptable because these methods are optimistic, not critical. Additionally, the other methods, preconntect, prefetchDNS, and preload, are not impacted because they already operated at the level of the ownerDocument and really only interface with the Network cache layer. I added a couple additional fixes that were necessary for getting tests to pass that are worth considering separately. The first commit improves the diff for `waitForThrow` so it compares strings if possible. The second commit makes invokeGuardedCallback not use metaprogramming pattern and swallows any novel errors produced from trying to run the guarded callback. Swallowing may not be the best we can do but it at least protects React against rapid failure when something causes the dispatchEvent to throw.
InvokeGuardedCallback is now implemented with the browser fork done at error-time rather than module-load-time. Originally it also tried to freeze the window/document references to avoid mismatches in prototype chains when testing React in different documents however we have since updated our tests to not do this and it was a test only feature so I removed it.
looks better now 🚀
This was added during an upgrade to Jest 24 in facebook#15778 By now we're at Jest 29. I think if CI passes we might not need this hack anymore.
This is the next step toward full support for async form actions. Errors thrown inside form actions should cause the form to re-render and throw the error so it can be captured by an error boundary. The behavior is the same if the `<form />` had an internal useTransition hook, which is pretty much exactly how we implement it, too. The first time an action is called, the form's HostComponent is "upgraded" to become stateful, by lazily mounting a list of hooks. The rest of the implementation for function components can be shared. Because the error handling behavior added in this commit is just using useTransition under-the-hood, it also handles pending states, too. However, this pending state can't be observed until we add a new hook for that purpose. I'll add this next.
This is stable and appears to build w/o problem. I don't see why we should disallow it.
Errors in form actions are now rethrown during render (facebook#26689), so we can handle them using an error boundary.
In React DOM, we use HostContext to represent the namespace of whatever is currently rendering — SVG, Math, or HTML. Because there is a fixed set of possible values, we can switch this to be a number instead. My motivation is that I want to start tracking additional information in this type, and I want to pack all of it into a single number instead of turning it into an object. For better performance. (In dev, the host context type is already an object that includes additional information, but that's dev so who cares.) Technically, before this change, the host context could be any namespace URI string, but any value other than SVG or Math was treated the same way. Only SVG and Math have special behavior. So in the new structure, there are three enum values: SVG, Math, or None, which represents the HTML namespace as well as all other possible namespaces.
Stacked on facebook#26557 Supporting Float methods such as ReactDOM.preload() are challenging for flight because it does not have an easy means to convey direct executions in other environments. Because the flight wire format is a JSON-like serialization that is expected to be rendered it currently only describes renderable elements. We need a way to convey a function invocation that gets run in the context of the client environment whether that is Fizz or Fiber. Fiber is somewhat straightforward because the HostDispatcher is always active and we can just have the FlightClient dispatch the serialized directive. Fizz is much more challenging becaue the dispatcher is always scoped but the specific request the dispatch belongs to is not readily available. Environments that support AsyncLocalStorage (or in the future AsyncContext) we will use this to be able to resolve directives in Fizz to the appropriate Request. For other environments directives will be elided. Right now this is pragmatic and non-breaking because all directives are opportunistic and non-critical. If this changes in the future we will need to reconsider how widespread support for async context tracking is. For Flight, if AsyncLocalStorage is available Float methods can be called before and after await points and be expected to work. If AsyncLocalStorage is not available float methods called in the sync phase of a component render will be captured but anything after an await point will be a noop. If a float call is dropped in this manner a DEV warning should help you realize your code may need to be modified. This PR also introduces a way for resources (Fizz) and hints (Flight) to flush even if there is not active task being worked on. This will help when Float methods are called in between async points within a function execution but the task is blocked on the entire function finishing. This PR also introduces deduping of Hints in Flight using the same resource keys used in Fizz. This will help shrink payload sizes when the same hint is attempted to emit over and over again
Use the Blob constructor + append with filename instead of File constructor. Node.js doesn't expose a global File constructor but does support it in this form. Queue fields until we get the 'end' event from the previous file. We rely on previous files being available by the time a field is resolved. However, since the 'end' event in Readable is fired after two micro-tasks, these are not resolved in order. I use a queue of the fields while we're still waiting on files to finish. This still doesn't resolve files and fields in order relative to each other but that doesn't matter for our usage.
This has been statically enabled everywhere for months.
…ebook#26708) Fizz can emit whatever it wants for the SSR version of these fields when it's a function action so they might not align with what is in the previous props. Therefore we need to force them to update if we're updating to a non-function where they might be relevant again.
JSON.stringify isn't the right thing here. Luckily this doesn't look to have any security impact.
…ebook#26715) The Promise as a child case seems buggy. It ends up throwing the Promise as fatal when used in Sync rendering.
facebook#26714) Insert temporary input node to polyfill submitter argument in FormData. This works for buttons too and fixes a bug where the type attribute wasn't reset. I also exclude the submitter if it's a function action. This ensures that we don't include the generated "name" when the action is a server action. Conceptually that name doesn't exist.
facebook#26720) This is consistent with what we used to do but not what we want to do.
This wires up, but does not yet implement, an experimental hook called useFormStatus. The hook is imported from React DOM, not React, because it represents DOM-specific state — its return type includes FormData as one of its fields. Other renderers that implement similar methods would use their own renderer-specific types. The API is prefixed and only available in the experimental channel. It can only be used from client (browser, SSR) components, not Server Components.
We currently use rollup to make an adhoc bundle from the file system when we're testing an import of an external file. This doesn't follow all the interception rules that we use in jest and in our actual builds. This switches to just using jest require() to load these. This means that they effectively have to load into the global document so this only works with global document tests which is all we have now anyway.
We used to have Event Replaying for any kind of Discrete event where we'd track any event after hydrateRoot and before the async code/data has loaded in to hydrate the target. However, this didn't really work out because code inside event handlers are expected to be able to synchronously read the state of the world at the time they're invoked. If we replay discrete events later, the mutable state around them like selection or form state etc. may have changed. This limitation doesn't apply to Client Actions: - They're expected to be async functions that themselves work asynchronously. They're conceptually also in the "navigation" events that happen after the "submit" events so they're already not synchronously even before the first `await`. - They're expected to operate mostly on the FormData as input which we can snapshot at the time of the event. This PR adds a bit of inline script to the Fizz runtime (or external runtime) to track any early submit events on the page - but only if the action URL is our placeholder `javascript:` URL. We track a queue of these on `document.$$reactFormReplay`. Then we replay them in order as they get hydrated and we get a handle on the Client Action function. I add the runtime to the `bootstrapScripts` phase in Fizz which is really technically a little too late, because on a large page, it might take a while to get to that script even if you have displayed the form. However, that's also true for external runtime. So there's a very short window we might miss an event but it's good enough and better than risking blocking display on this script. The main thing that makes the replaying difficult to reason about is that we can have multiple instance of React using this same queue. This would be very usual but you could have two different Reacts SSR:ing different parts of the tree and using around the same version. We don't have any coordinating ids for this. We could stash something on the form perhaps but given our current structure it's more difficult to get to the form instance in the commit phase and a naive solution wouldn't preserve ordering between forms. This solution isn't 100% guaranteed to preserve ordering between different React instances neither but should be in order within one instance which is the common case. The hard part is that we don't know what instance something will belong to until it hydrates. So to solve that I keep everything in the original queue while we wait, so that ordering is preserved until we know which instance it'll go into. I ended up doing a bunch of clever tricks to make this work. These could use a lot more tests than I have right now. Another thing that's tricky is that you can update the action before it's replayed but we actually want to invoke the old action if that happens. So we have to extract it even if we can't invoke it right now just so we get the one that was there during hydration.
This is enabled in the canary channels, but because it's relatively untested, we'll disable it at Meta until they're ready to start trying it out. It can change some behavior even if you don't intentionally start using the API. The reason it's not a dynamic flag is that it affects the external Fizz runtime, which currently can't read flags at runtime.
useMemoCache wasn't previously supported in the DevTools, so any attempt to inspect a component using the hook would result in a `dispatcher.useMemoCache is not a function (it is undefined)` error.
…book#26727) ## Summary We added some post-processing in the build for RN in facebook#26616 that broke for users on Windows due to how line endings were handled to the regular expression to insert some directives in the docblock. This fixes that problem, reported in facebook#26697 as well. ## How did you test this change? Verified files are still built correctly on Mac/Linux. Will ask for help to test on Windows.
I found a couple scenarios where preloads were issued too aggressively 1. During SSR, if you render a new stylesheet after the preamble flushed it will flush a preload even if the resource was already preloaded 2. During Client render, if you call `ReactDOM.preload()` it will only check if a preload exists in the Document before inserting a new one. It should check for an underlying resource such as a stylesheet link or script if the preload is for a recognized asset type
When there are multiple async actions at the same time, we entangle them together because we can't be sure which action an update might be associated with. (For this, we'd need AsyncContext.) However, if one of the async actions fails with an error, it should only affect that action, not all the other actions it may be entangled with. Resolving each action independently also means they can have independent pending state types, rather than being limited to an `isPending` boolean. We'll use this to implement an upcoming form API.
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.
No description provided.