Skip to content
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.

Sign up for GitHub

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

[Flight] Add BigInt support #26479

Merged
merged 4 commits into from
Mar 29, 2023
Merged

[Flight] Add BigInt support #26479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
604250c
[Flight] Add support BigInt support
eps1lon Mar 25, 2023
3a08f77
Fix lint-build
eps1lon Mar 26, 2023
fa40187
comment why we need `bigint`
eps1lon Mar 28, 2023
849adf3
Use base 10 for better debuggability
eps1lon Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,9 @@ module.exports = {
$ReadOnlyArray: 'readonly',
$Shape: 'readonly',
AnimationFrameID: 'readonly',
// For Flow type annotation. Only `BigInt` is valid at runtime.
bigint: 'readonly',
Copy link
Collaborator

Choose a reason for hiding this comment

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

😮 Is this due to the type annotation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I remember correctly, yes. I'll double check

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess there's a bunch of types in here already. Which is unfortunate since it'll cover up mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At least we would catch mistakes for bundles that we lint since we don't have bigint the configs for linting bundles.

I could inline serializeBigint?

In TypeScript projects we just disable this rule and let TypeScript check for usage of undefined variables. Don't know how we could do this for files being typechecked by Flow. /cc @kassens

Copy link
Member

Choose a reason for hiding this comment

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

Not sure how we could have eslint just ignore flow files. I think it's only easy by the full file name.

Good thing is that Flow will still catch bigint as undefined regardless of the config here, so really only non-flow files are a bit at risk. Seems okay to me.

BigInt: 'readonly',
Class: 'readonly',
ClientRect: 'readonly',
CopyInspectedElementPath: 'readonly',
Expand Down
4 changes: 4 additions & 0 deletions packages/react-client/src/ReactFlightClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,10 @@ export function parseModelString(
// Special encoding for `undefined` which can't be serialized as JSON otherwise.
return undefined;
}
case 'n': {
// BigInt
return BigInt(value.substring(2));
}
default: {
// We assume that anything else is a reference ID.
const id = parseInt(value.substring(1), 16);
Expand Down
8 changes: 5 additions & 3 deletions packages/react-client/src/ReactFlightReplyClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ function serializeUndefined(): string {
return '$undefined';
}

function serializeBigInt(n: bigint): string {
return '$n' + n.toString(10);
}

function escapeStringValue(value: string): string {
if (value[0] === '$') {
// We need to escape $ prefixed strings since we use those to encode
Expand Down Expand Up @@ -264,9 +268,7 @@ export function processReply(
}

if (typeof value === 'bigint') {
throw new Error(
`BigInt (${value}) is not yet supported as an argument to a Server Function.`,
);
return serializeBigInt(value);
}

throw new Error(
Expand Down
19 changes: 19 additions & 0 deletions packages/react-client/src/__tests__/ReactFlight-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,25 @@ describe('ReactFlight', () => {
expect(ReactNoop).toMatchRenderedOutput(null);
});

it('can transport BigInt', async () => {
function ComponentClient({prop}) {
return `prop: ${prop} (${typeof prop})`;
}
const Component = clientReference(ComponentClient);

const model = <Component prop={90071992547409910000n} />;

const transport = ReactNoopFlightServer.render(model);

await act(async () => {
ReactNoop.render(await ReactNoopFlightClient.read(transport));
});

expect(ReactNoop).toMatchRenderedOutput(
'prop: 90071992547409910000 (bigint)',
);
});

it('can render a lazy component as a shared component on the server', async () => {
function SharedComponent({text}) {
return (
Expand Down
7 changes: 7 additions & 0 deletions packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,11 @@ describe('ReactFlightDOMReply', () => {
}
expect(items).toEqual(['A', 'B', 'C']);
});

it('can pass a BigInt as a reply', async () => {
const body = await ReactServerDOMClient.encodeReply(90071992547409910000n);
const n = await ReactServerDOMServer.decodeReply(body, webpackServerMap);

expect(n).toEqual(90071992547409910000n);
});
});
4 changes: 4 additions & 0 deletions packages/react-server/src/ReactFlightReplyServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,10 @@ function parseModelString(
// Special encoding for `undefined` which can't be serialized as JSON otherwise.
return undefined;
}
case 'n': {
// BigInt
return BigInt(value.substring(2));
}
default: {
// We assume that anything else is a reference ID.
const id = parseInt(value.substring(1), 16);
Expand Down
9 changes: 5 additions & 4 deletions packages/react-server/src/ReactFlightServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,10 @@ function serializeUndefined(): string {
return '$undefined';
}

function serializeBigInt(n: bigint): string {
return '$n' + n.toString(10);
}

function serializeClientReference(
request: Request,
parent:
Expand Down Expand Up @@ -931,10 +935,7 @@ export function resolveModelToJSON(
}

if (typeof value === 'bigint') {
throw new Error(
`BigInt (${value}) is not yet supported in Client Component props.` +
describeObjectForErrorMessage(parent, key),
);
return serializeBigInt(value);
}

throw new Error(
Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/validate/eslintrc.esm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
},
globals: {
// ES 6
BigInt: 'readonly',
Map: 'readonly',
Set: 'readonly',
Proxy: 'readonly',
Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/validate/eslintrc.fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
},
globals: {
// ES6
BigInt: 'readonly',
Map: 'readonly',
Set: 'readonly',
Symbol: 'readonly',
Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/validate/eslintrc.rn.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module.exports = {
},
globals: {
// ES6
BigInt: 'readonly',
Map: 'readonly',
Set: 'readonly',
Symbol: 'readonly',
Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/validate/eslintrc.umd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module.exports = {
},
globals: {
// ES6
BigInt: 'readonly',
Map: 'readonly',
Set: 'readonly',
Symbol: 'readonly',
Expand Down