Skip to content

Commit

Permalink
fix(core): unchained commands should use a new transaction. Fixes rem…
Browse files Browse the repository at this point in the history
  • Loading branch information
whawker authored Sep 7, 2021
1 parent 7cba6fb commit 8e4398c
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 50 deletions.
5 changes: 5 additions & 0 deletions .changeset/twelve-hotels-matter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@remirror/core': patch
---

Unchained commands should use a new transaction to prevent leaking of previous command steps
13 changes: 9 additions & 4 deletions packages/remirror__core/__tests__/remirror-manager.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import type {
ProsemirrorAttributes,
} from '@remirror/core-types';
import { Schema } from '@remirror/pm/model';
import { EditorState, Plugin } from '@remirror/pm/state';
import { EditorState, Plugin, Transaction } from '@remirror/pm/state';
import { EditorView } from '@remirror/pm/view';

describe('Manager', () => {
Expand Down Expand Up @@ -95,14 +95,19 @@ describe('Manager', () => {
it('supports commands', () => {
const attributes = { a: 'a' };
manager.store.commands.dummy(attributes);
const { state, dispatch } = manager.view;

expect(mock).toHaveBeenCalledWith(attributes);
expect(innerMock).toHaveBeenCalledWith({
state: manager.view.state,
dispatch: manager.view.dispatch,
state: state,
dispatch: dispatch,
view: manager.view,
tr: manager.tr,
tr: expect.any(Transaction),
});

const { tr } = innerMock.mock.calls[0][0];
// Check this transaction is empty
expect(tr.steps).toHaveLength(0);
});

it('supports helpers', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/remirror__core/src/builtins/commands-extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,7 @@ export class CommandsExtension extends PlainExtension<CommandOptions> {
dispatch = view.dispatch;
}

return command(...args)({ state, dispatch, view, tr: this.transaction });
return command(...args)({ state, dispatch, view, tr: state.tr });
};
}

Expand Down
16 changes: 1 addition & 15 deletions packages/remirror__react-core/__tests__/controlled.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -329,22 +329,8 @@ test('can run multiple commands', () => {
};

const Component = () => {
const [value, setValue] = useState<EditorState>(
manager.createState({
content: '',
}),
);

return (
<Remirror
{...props}
state={value}
manager={manager}
onChange={(changeProps) => {
const { state } = changeProps;
setValue(state);
}}
>
<Remirror {...props} manager={manager}>
<InnerComponent />
</Remirror>
);
Expand Down
35 changes: 6 additions & 29 deletions packages/remirror__react-core/__tests__/core.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import { axe } from 'jest-axe';
import { RemirrorTestChain } from 'jest-remirror';
import { useCallback, useState } from 'react';
import { useState } from 'react';
import { renderToString } from 'react-dom/server';
import { RemirrorEventListener } from 'remirror';
import { hideConsoleError, rafMock } from 'testing';
import { act, fireEvent, render, strictRender } from 'testing/react';
import {
createReactManager,
ReactExtensions,
ReactFrameworkOutput,
Remirror,
useRemirror,
Expand Down Expand Up @@ -338,34 +336,13 @@ test('`focus` should be chainable', () => {
};

const Component = () => {
const [state, setState] = useState(() =>
manager.createState({
content: '<p>Content </p>',
selection: 'end',
}),
);

const onChange: RemirrorEventListener<ReactExtensions<never>> = useCallback(
({ state, firstRender }) => {
if (firstRender) {
return;
}

act(() => {
setState(state);
});
},
[],
);
const state = manager.createState({
content: '<p>Content </p>',
selection: 'end',
});

return (
<Remirror
autoFocus={true}
manager={manager}
onChange={onChange}
state={state}
autoRender='start'
>
<Remirror autoFocus={true} manager={manager} initialContent={state} autoRender='start'>
<TrapContext />
</Remirror>
);
Expand Down
2 changes: 1 addition & 1 deletion support/root/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ module.exports = {
'!packages/remirror__cli/**',
],
coverageReporters: ['json', 'lcov', 'text-summary', 'clover'],
collectCoverage: true,
collectCoverage: !!process.env.CI,
reporters,
watchPlugins: ['jest-watch-typeahead/filename', 'jest-watch-typeahead/testname'],
testRunner: 'jest-circus/runner',
Expand Down

0 comments on commit 8e4398c

Please sign in to comment.