Skip to content

Commit

Permalink
fix(react): prevent multiple attached editors
Browse files Browse the repository at this point in the history
  • Loading branch information
ifiokjr committed Jul 13, 2020
1 parent 2c7c9ac commit e1a1b6e
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 37 deletions.
5 changes: 5 additions & 0 deletions .changeset/hungry-cherries-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@remirror/react': patch
---

Prevent multiple editors being attached with a single Provider. This error flags you when you are attaching `getRootProps` to the dom in multiple placeswithin a single editor instance. This can help prevent unwanted behaviour.
7 changes: 4 additions & 3 deletions docs/errors.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,15 @@ Extra attributes must either be a string or an object.

> React Provider Context
`useRemirror` was called outside of the remirror context. It can only be used within an active
remirror context created by the `<RemirrorProvider />`.
**`useRemirror` was called outside of the remirror context. It can only be used within an active
remirror context created by the `<RemirrorProvider />`.**

### RMR0201

> React Get Root Props
`getRootProps` has been called MULTIPLE times. It should only be called ONCE during render.
`getRootProps` has been attached to the DOM more than once. It should only be attached to the DOM
**once** per editor.

This error happens because the `getRootProps` method is being used in a component that is rendered
multiple times without updates from the react provider. The `getRootProps` method is responsible for
Expand Down
2 changes: 1 addition & 1 deletion packages/@remirror/core-helpers/src/core-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ if (process.env.NODE_ENV !== 'production') {
[ErrorConstant.REACT_PROVIDER_CONTEXT]:
'`useRemirror` was called outside of the `remirror` context. It can only be used within an active remirror context created by the `<RemirrorProvider />`.',
[ErrorConstant.REACT_GET_ROOT_PROPS]:
'`getRootProps` has been called MULTIPLE times. It should only be called ONCE during render.',
'`getRootProps` has been attached to the DOM more than once. It should only be attached to the dom once per editor.',
[ErrorConstant.REACT_EDITOR_VIEW]: 'A problem occurred adding the editor view to the dom.',
[ErrorConstant.REACT_CONTROLLED]: 'There is a problem with your controlled editor setup.',
[ErrorConstant.I18N_CONTEXT]: 'You called `useI18n()` outside of an `I18nProvider` context.',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`multiple \`getRootProps\` applied to dom throw an error 1`] = `
"\`getRootProps\` has been attached to the DOM more than once. It should only be attached to the dom once per editor.
Called 2 times
For more information visit https://remirror.io/docs/errors#rmr0201"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React, { FC } from 'react';

import { docNodeBasicJSON } from '@remirror/testing';
import { createReactManager, render } from '@remirror/testing/react';

import { useRemirror } from '../../hooks';
import { RemirrorProvider } from '../providers';

test('`RemirrorProvider`', () => {
const TestComponent: FC = () => {
const { getRootProps } = useRemirror();
return (
<div>
<div data-testid='target' {...getRootProps()} />
</div>
);
};

const manager = createReactManager([]);

const { getByRole, getByTestId } = render(
<RemirrorProvider initialContent={docNodeBasicJSON} manager={manager}>
<TestComponent />
</RemirrorProvider>,
);
const target = getByTestId('target');
const editor = getByRole('textbox');

expect(target).toContainElement(editor);
});

test('multiple `getRootProps` applied to dom throw an error', () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => {});
const manager = createReactManager([]);
const TestComponent: FC = () => {
const { getRootProps } = useRemirror();

return (
<div>
<div data-testid='target' {...getRootProps()} />
<div data-testid='target1' {...getRootProps()} />
</div>
);
};

expect(() =>
render(
<RemirrorProvider manager={manager}>
<TestComponent />
</RemirrorProvider>,
),
).toThrowErrorMatchingSnapshot();

spy.mockRestore();
});
Original file line number Diff line number Diff line change
@@ -1,33 +1,8 @@
import React, { FC } from 'react';
import React from 'react';

import { docNodeBasicJSON } from '@remirror/testing';
import { createReactManager, render } from '@remirror/testing/react';
import { render } from '@remirror/testing/react';

import { useRemirror } from '../../hooks';
import { RemirrorProvider, ThemeProvider } from '../providers';

test('RemirrorProvider', () => {
const TestComponent: FC = () => {
const { getRootProps } = useRemirror();
return (
<div>
<div data-testid='target' {...getRootProps()} />
</div>
);
};

const manager = createReactManager([]);

const { getByRole, getByTestId } = render(
<RemirrorProvider initialContent={docNodeBasicJSON} manager={manager}>
<TestComponent />
</RemirrorProvider>,
);
const target = getByTestId('target');
const editor = getByRole('textbox');

expect(target).toContainElement(editor);
});
import { ThemeProvider } from '../providers';

describe('ThemeProvider', () => {
it('should render', () => {
Expand Down
19 changes: 15 additions & 4 deletions packages/@remirror/react/src/components/react-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ class ReactEditorWrapper<Combined extends AnyCombinedUnion> extends EditorWrappe
*/
private rootPropsConfig = {
called: false,
mounts: 0,
};

constructor(parameter: ReactEditorWrapperParameter<Combined>) {
Expand Down Expand Up @@ -285,7 +286,7 @@ class ReactEditorWrapper<Combined extends AnyCombinedUnion> extends EditorWrappe
children?: ReactNode,
): RefKeyRootProps<RefKey> => {
// Ensure that this is the first time `getRootProps` is being called during
// this render.
// this commit phase of the .
// invariant(!this.rootPropsConfig.called, { code: ErrorConstant.REACT_GET_ROOT_PROPS });
this.rootPropsConfig.called = true;

Expand All @@ -303,10 +304,19 @@ class ReactEditorWrapper<Combined extends AnyCombinedUnion> extends EditorWrappe
* Stores the Prosemirror editor dom instance for this component using `refs`
*/
private readonly onRef: Ref<HTMLElement> = (element) => {
if (element) {
this.#editorRef = element;
this.onRefLoad();
if (!element) {
return;
}

this.rootPropsConfig.mounts += 1;

invariant(this.rootPropsConfig.mounts <= 1, {
code: ErrorConstant.REACT_GET_ROOT_PROPS,
message: `Called ${this.rootPropsConfig.mounts} times`,
});

this.#editorRef = element;
this.onRefLoad();
};

/**
Expand Down Expand Up @@ -482,6 +492,7 @@ class ReactEditorWrapper<Combined extends AnyCombinedUnion> extends EditorWrappe
private resetRender() {
// Reset the status of roots props being called
this.rootPropsConfig.called = false;
this.rootPropsConfig.mounts = 0;
}

render() {
Expand Down
2 changes: 1 addition & 1 deletion support/jest/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const { jestSupportDir, baseDir } = require('./helpers');

const { TEST_BUILD } = process.env;

/** @type Partial<import("@jest/types").Config.GlobalConfig> */
/** @type Partial<import("@jest/types").Config.InitialOptions> */
module.exports = {
clearMocks: true,
verbose: true,
Expand Down

0 comments on commit e1a1b6e

Please sign in to comment.