Skip to content

Conversation

sebmarkbage
Copy link
Collaborator

We decremented allPendingTasks after invoking onShellReady. Which means that in that scope it wasn't considered fully complete.

Since the pattern for flushing in Node.js is to start piping in onShellReady and that's how you can get sync behavior, this led us to think that we had more work left to do. For example we emitted the writeShellTimeInstruction in this scenario before.

@sebmarkbage sebmarkbage requested a review from gnoff May 16, 2025 18:20
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label May 16, 2025
@react-sizebot
Copy link

Comparing: 4448b18...0c80167

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 529.74 kB 529.74 kB = 93.49 kB 93.49 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 651.48 kB 651.48 kB = 114.76 kB 114.76 kB
facebook-www/ReactDOM-prod.classic.js = 675.72 kB 675.72 kB = 118.85 kB 118.85 kB
facebook-www/ReactDOM-prod.modern.js = 666.00 kB 666.00 kB = 117.23 kB 117.23 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 0c80167

},
);
});
expect(output.result).toMatchInlineSnapshot(`"<div>hello world</div>"`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could even move this into the onShellReady callback after the pipe. I think it will flush everything sync

@sebmarkbage sebmarkbage merged commit c250b7d into facebook:main May 16, 2025
246 checks passed
github-actions bot pushed a commit that referenced this pull request May 16, 2025
…3295)

We decremented `allPendingTasks` after invoking `onShellReady`. Which
means that in that scope it wasn't considered fully complete.

Since the pattern for flushing in Node.js is to start piping in
`onShellReady` and that's how you can get sync behavior, this led us to
think that we had more work left to do. For example we emitted the
`writeShellTimeInstruction` in this scenario before.

DiffTrain build for [c250b7d](c250b7d)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 17, 2025
…cebook#33295)

We decremented `allPendingTasks` after invoking `onShellReady`. Which
means that in that scope it wasn't considered fully complete.

Since the pattern for flushing in Node.js is to start piping in
`onShellReady` and that's how you can get sync behavior, this led us to
think that we had more work left to do. For example we emitted the
`writeShellTimeInstruction` in this scenario before.

DiffTrain build for [c250b7d](facebook@c250b7d)
github-actions bot pushed a commit to addcx1developer/react that referenced this pull request May 17, 2025
…cebook#33295)

We decremented `allPendingTasks` after invoking `onShellReady`. Which
means that in that scope it wasn't considered fully complete.

Since the pattern for flushing in Node.js is to start piping in
`onShellReady` and that's how you can get sync behavior, this led us to
think that we had more work left to do. For example we emitted the
`writeShellTimeInstruction` in this scenario before.

DiffTrain build for [c250b7d](facebook@c250b7d)
github-actions bot pushed a commit to addcx1developer/react that referenced this pull request May 17, 2025
…cebook#33295)

We decremented `allPendingTasks` after invoking `onShellReady`. Which
means that in that scope it wasn't considered fully complete.

Since the pattern for flushing in Node.js is to start piping in
`onShellReady` and that's how you can get sync behavior, this led us to
think that we had more work left to do. For example we emitted the
`writeShellTimeInstruction` in this scenario before.

DiffTrain build for [c250b7d](facebook@c250b7d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants