Skip to content

Commit

Permalink
fix(ssr): use useId for hydration and remove the need for resetServer…
Browse files Browse the repository at this point in the history
…Context (React 18+) (#439)

* feat(ssr): remove need for resetServerContext when useId is available (i.e. React 18+)

* refactor: add deprecated prefix to some functions (with test utils)

* test(process.env): add missing types for the csp-server

Co-authored-by: klarstrup <[email protected]>
  • Loading branch information
100terres and klarstrup authored Nov 26, 2022
1 parent 3e33920 commit bcb66d3
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 36 deletions.
22 changes: 11 additions & 11 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
{
"dnd.js": {
"bundled": 312882,
"minified": 119680,
"gzipped": 36562
"bundled": 313453,
"minified": 119985,
"gzipped": 36652
},
"dnd.min.js": {
"bundled": 275071,
"minified": 101956,
"gzipped": 30151
"bundled": 275553,
"minified": 102189,
"gzipped": 30211
},
"dnd.esm.js": {
"bundled": 217232,
"minified": 121547,
"gzipped": 32474,
"bundled": 217729,
"minified": 121864,
"gzipped": 32558,
"treeshaked": {
"rollup": {
"code": 18507,
"code": 18649,
"import_statements": 456
},
"webpack": {
"code": 21510
"code": 21689
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions csp-server/environment.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
interface ProcessEnv {
NODE_ENV?: 'development' | 'production';
REACT_MAJOR_VERSION?: '16' | '17' | '18';
CI?: boolean;
}

interface Process {
env: ProcessEnv;
}
5 changes: 4 additions & 1 deletion csp-server/server.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import { renderToString } from 'react-dom/server';
import { resetServerContext } from '@hello-pangea/dnd';
import { resolve } from 'path';
import App from './app';
import invokeOnReactVersion from '../test/util/invoke-on-react-version';

let count = 0;
function getNonce(): string {
return `ThisShouldBeACryptographicallySecurePseudoRandomNumber-${count++}`;
}

function renderHtml(policy?: string, nonce?: string) {
resetServerContext();
invokeOnReactVersion(['16', '17'], resetServerContext);

let meta = '';
if (nonce) {
meta += `<meta id="csp-nonce" property="csp-nonce" content="${nonce}" />`;
Expand All @@ -32,6 +34,7 @@ export default (port: string, outputPath: string, fs: any): void => {
res.end(fs.readFileSync(resolve(outputPath, 'client.js')));
});

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function render(res: any, policy?: string, nonce?: string) {
if (policy) {
res.header('Content-Security-Policy', policy);
Expand Down
6 changes: 6 additions & 0 deletions jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ if (['16', '17'].includes(reactMajorVersion)) {
'^react-dom((\\/.*)?)$': `react-dom-${reactMajorVersion}$1`,
'^react((\\/.*)?)$': `react-${reactMajorVersion}$1`,
};
} else {
config.testPathIgnorePatterns = [
...(config.testPathIgnorePatterns || []),
// resetServerContext is irrelevant from 18 onwards
'test/unit/integration/drag-drop-context/reset-server-context.spec.tsx',
];
}

if (isRunningInCI()) {
Expand Down
18 changes: 14 additions & 4 deletions src/view/drag-drop-context/drag-drop-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ import React from 'react';
import type { ReactNode } from 'react';
import type { Responders, ContextId, Sensor } from '../../types';
import ErrorBoundary from './error-boundary';
import { warning } from '../../dev-warning';
import preset from '../../screen-reader-message-preset';
import App from './app';
import useUniqueContextId, {
reset as resetContextId,
resetDeprecatedUniqueContextId,
} from './use-unique-context-id';
import { reset as resetUniqueIds } from '../use-unique-id';
import { resetDeprecatedUniqueId } from '../use-unique-id';
import { PartialAutoScrollerOptions } from '../../state/auto-scroller/fluid-scroller/auto-scroller-options-types';

export interface DragDropContextProps extends Responders {
Expand All @@ -29,8 +30,17 @@ export interface DragDropContextProps extends Responders {

// Reset any context that gets persisted across server side renders
export function resetServerContext() {
resetContextId();
resetUniqueIds();
// The useId hook is only available in React 18+
if ('useId' in React) {
warning(
`It is not necessary to call resetServerContext when using React 18+`,
);

return;
}

resetDeprecatedUniqueContextId();
resetDeprecatedUniqueId();
}

export default function DragDropContext(props: DragDropContextProps) {
Expand Down
14 changes: 12 additions & 2 deletions src/view/drag-drop-context/use-unique-context-id.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
import React from 'react';
import { useMemo } from 'use-memo-one';
import type { ContextId } from '../../types';

let count = 0;

export function reset() {
export function resetDeprecatedUniqueContextId() {
count = 0;
}

export default function useInstanceCount(): ContextId {
function useDeprecatedUniqueContextId(): ContextId {
return useMemo(() => `${count++}`, []);
}

function useUniqueContextId(): ContextId {
return React.useId();
}

// The useId hook is only available in React 18+
export default 'useId' in React
? useUniqueContextId
: useDeprecatedUniqueContextId;
17 changes: 15 additions & 2 deletions src/view/use-unique-id.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import React from 'react';
import { useMemo } from 'use-memo-one';
import type { Id } from '../types';

Expand All @@ -9,11 +10,11 @@ interface Options {

const defaults: Options = { separator: '::' };

export function reset() {
export function resetDeprecatedUniqueId() {
count = 0;
}

export default function useUniqueId(
function useDeprecatedUniqueId(
prefix: string,
options: Options = defaults,
): Id {
Expand All @@ -22,3 +23,15 @@ export default function useUniqueId(
[options.separator, prefix],
);
}

function useUniqueId(prefix: string, options: Options = defaults): Id {
const id = React.useId();

return useMemo(
() => `${prefix}${options.separator}${id}`,
[options.separator, prefix, id],
);
}

// The useId hook is only available in React 18+
export default 'useId' in React ? useUniqueId : useDeprecatedUniqueId;
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`should support rendering to a string 1`] = `"<main><div data-rfd-droppable-id="droppable" data-rfd-droppable-context-id="0"><div data-rfd-draggable-context-id="0" data-rfd-draggable-id="0" tabindex="0" role="button" aria-describedby="rfd-hidden-text-0-hidden-text-0" data-rfd-drag-handle-draggable-id="0" data-rfd-drag-handle-context-id="0" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="0">item: <!-- -->0</div><div data-rfd-draggable-context-id="0" data-rfd-draggable-id="1" tabindex="0" role="button" aria-describedby="rfd-hidden-text-0-hidden-text-0" data-rfd-drag-handle-draggable-id="1" data-rfd-drag-handle-context-id="0" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="1">item: <!-- -->1</div><div data-rfd-draggable-context-id="0" data-rfd-draggable-id="2" tabindex="0" role="button" aria-describedby="rfd-hidden-text-0-hidden-text-0" data-rfd-drag-handle-draggable-id="2" data-rfd-drag-handle-context-id="0" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="2">item: <!-- -->2</div></div></main>"`;
exports[`should support rendering to a string 1`] = `"<main><div data-rfd-droppable-id="droppable" data-rfd-droppable-context-id=":R0:"><div data-rfd-draggable-context-id=":R0:" data-rfd-draggable-id="0" tabindex="0" role="button" aria-describedby="rfd-hidden-text-:R0:-hidden-text-:R1:" data-rfd-drag-handle-draggable-id="0" data-rfd-drag-handle-context-id=":R0:" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="0">item: <!-- -->0</div><div data-rfd-draggable-context-id=":R0:" data-rfd-draggable-id="1" tabindex="0" role="button" aria-describedby="rfd-hidden-text-:R0:-hidden-text-:R1:" data-rfd-drag-handle-draggable-id="1" data-rfd-drag-handle-context-id=":R0:" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="1">item: <!-- -->1</div><div data-rfd-draggable-context-id=":R0:" data-rfd-draggable-id="2" tabindex="0" role="button" aria-describedby="rfd-hidden-text-:R0:-hidden-text-:R1:" data-rfd-drag-handle-draggable-id="2" data-rfd-drag-handle-context-id=":R0:" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="2">item: <!-- -->2</div></div></main>"`;

exports[`should support rendering to static markup 1`] = `"<main><div data-rfd-droppable-id="droppable" data-rfd-droppable-context-id="0"><div data-rfd-draggable-context-id="0" data-rfd-draggable-id="0" tabindex="0" role="button" aria-describedby="rfd-hidden-text-0-hidden-text-0" data-rfd-drag-handle-draggable-id="0" data-rfd-drag-handle-context-id="0" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="0">item: 0</div><div data-rfd-draggable-context-id="0" data-rfd-draggable-id="1" tabindex="0" role="button" aria-describedby="rfd-hidden-text-0-hidden-text-0" data-rfd-drag-handle-draggable-id="1" data-rfd-drag-handle-context-id="0" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="1">item: 1</div><div data-rfd-draggable-context-id="0" data-rfd-draggable-id="2" tabindex="0" role="button" aria-describedby="rfd-hidden-text-0-hidden-text-0" data-rfd-drag-handle-draggable-id="2" data-rfd-drag-handle-context-id="0" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="2">item: 2</div></div></main>"`;
exports[`should support rendering to static markup 1`] = `"<main><div data-rfd-droppable-id="droppable" data-rfd-droppable-context-id=":R0:"><div data-rfd-draggable-context-id=":R0:" data-rfd-draggable-id="0" tabindex="0" role="button" aria-describedby="rfd-hidden-text-:R0:-hidden-text-:R1:" data-rfd-drag-handle-draggable-id="0" data-rfd-drag-handle-context-id=":R0:" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="0">item: 0</div><div data-rfd-draggable-context-id=":R0:" data-rfd-draggable-id="1" tabindex="0" role="button" aria-describedby="rfd-hidden-text-:R0:-hidden-text-:R1:" data-rfd-drag-handle-draggable-id="1" data-rfd-drag-handle-context-id=":R0:" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="1">item: 1</div><div data-rfd-draggable-context-id=":R0:" data-rfd-draggable-id="2" tabindex="0" role="button" aria-describedby="rfd-hidden-text-:R0:-hidden-text-:R1:" data-rfd-drag-handle-draggable-id="2" data-rfd-drag-handle-context-id=":R0:" draggable="false" data-is-dragging="false" data-is-drop-animating="false" data-is-combining="false" data-is-combine-target="false" data-is-clone="false" data-testid="2">item: 2</div></div></main>"`;
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import { resetServerContext } from '../../../../src';
import App from '../util/app';
import { noop } from '../../../../src/empty';
import getBodyElement from '../../../../src/view/get-body-element';
import invokeOnReactVersion from '../../../util/invoke-on-react-version';

beforeEach(() => {
// Reset server context between tests to prevent state being shared between them
resetServerContext();
invokeOnReactVersion(['16', '17'], resetServerContext);
});

// Checking that the browser globals are available in this test file
Expand All @@ -25,7 +26,7 @@ it('should support hydrating a server side rendered application', () => {
// on the server
const error = jest.spyOn(console, 'error').mockImplementation(noop);

resetServerContext();
invokeOnReactVersion(['16', '17'], resetServerContext);
const serverHTML: string = ReactDOMServer.renderToString(<App />);

error.mock.calls.forEach((call) => {
Expand All @@ -37,7 +38,7 @@ it('should support hydrating a server side rendered application', () => {

// would be done client side
// would have a fresh server context on the client
resetServerContext();
invokeOnReactVersion(['16', '17'], resetServerContext);
const el = document.createElement('div');
el.innerHTML = serverHTML;
getBodyElement().appendChild(el);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import { renderToString, renderToStaticMarkup } from 'react-dom/server';
import { invariant } from '../../../../src/invariant';
import { resetServerContext } from '../../../../src';
import App from '../util/app';
import invokeOnReactVersion from '../../../util/invoke-on-react-version';

let consoleWarnSpy: jest.SpiedFunction<typeof console.warn>;
let consoleErrorSpy: jest.SpiedFunction<typeof console.error>;
let consoleLogSpy: jest.SpiedFunction<typeof console.log>;

beforeEach(() => {
// Reset server context between tests to prevent state being shared between them
resetServerContext();
invokeOnReactVersion(['16', '17'], resetServerContext);

consoleWarnSpy = jest.spyOn(console, 'warn');
consoleErrorSpy = jest.spyOn(console, 'error');
Expand Down Expand Up @@ -54,13 +55,17 @@ it('should support rendering to static markup', () => {
expectConsoleNotCalled();
});

it('should render identical content when resetting context between renders', () => {
const firstRender = renderToString(<App />);
const nextRenderBeforeReset = renderToString(<App />);
expect(firstRender).not.toEqual(nextRenderBeforeReset);
// This test is unsuited for React 18+, which with useId can
// produce simultaneously unique and deterministic IDs
invokeOnReactVersion(['16', '17'], () => {
it('should render identical content when resetting context between renders', () => {
const firstRender = renderToString(<App />);
const nextRenderBeforeReset = renderToString(<App />);
expect(firstRender).not.toEqual(nextRenderBeforeReset);

resetServerContext();
const nextRenderAfterReset = renderToString(<App />);
expect(firstRender).toEqual(nextRenderAfterReset);
expectConsoleNotCalled();
resetServerContext();
const nextRenderAfterReset = renderToString(<App />);
expect(firstRender).toEqual(nextRenderAfterReset);
expectConsoleNotCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,22 @@ import React from 'react';
import DragDropContext from '../../../../src/view/drag-drop-context';
import { resetServerContext } from '../../../../src';
import * as attributes from '../../../../src/view/data-attributes';
import getReactMajorVersion from '../../../util/get-react-major-version';
import invokeOnReactVersion from '../../../util/invoke-on-react-version';

const isReact18 = getReactMajorVersion() === '18';
it('should insert nonce into style tag', () => {
const nonce = 'ThisShouldBeACryptographicallySecurePseudorandomNumber';

resetServerContext();
invokeOnReactVersion(['16', '17'], resetServerContext);
const { unmount } = render(
<DragDropContext nonce={nonce} onDragEnd={() => {}}>
{null}
</DragDropContext>,
);
const styleTag = document.querySelector(`[${attributes.prefix}-always="0"]`);
const styleTag = document.querySelector(
`[${attributes.prefix}-always="${isReact18 ? ':r0:' : '0'}"]`,
);
const nonceAttribute = styleTag ? styleTag.getAttribute('nonce') : '';
expect(nonceAttribute).toEqual(nonce);

Expand Down
10 changes: 10 additions & 0 deletions test/util/invoke-on-react-version.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import getReactMajorVersion from './get-react-major-version';

export default function invokeOnReactVersion(
reactVersion: Array<'16' | '17' | '18'>,
callback: () => void,
) {
if (reactVersion.includes(getReactMajorVersion())) {
callback();
}
}

1 comment on commit bcb66d3

@vercel
Copy link

@vercel vercel bot commented on bcb66d3 Nov 26, 2022

Choose a reason for hiding this comment

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

Please sign in to comment.