Skip to content

Commit

Permalink
fix: prevent expressions from recomputing more than once
Browse files Browse the repository at this point in the history
Closes #1151
  • Loading branch information
Skaiir committed Apr 16, 2024
1 parent a17040a commit 7565fc3
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 9 deletions.
20 changes: 17 additions & 3 deletions packages/form-js-viewer/src/Form.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import Ids from 'ids';
import { get, isObject, isString, isUndefined, set } from 'min-dash';

import { ExpressionLanguageModule, MarkdownRendererModule, ViewerCommandsModule, RepeatRenderModule } from './features';
import {
ExpressionLanguageModule,
ExpressionFieldModule,
MarkdownRendererModule,
ViewerCommandsModule,
RepeatRenderModule,
} from './features';

import { CoreModule } from './core';

Expand Down Expand Up @@ -317,7 +323,7 @@ export class Form {
/**
* @internal
*
* @param { { add?: boolean, field: any, indexes: object, remove?: number, value?: any } } update
* @param { { field: any, indexes: object, value: any } } update
*/
_update(update) {
const { field, indexes, value } = update;
Expand All @@ -335,6 +341,8 @@ export class Form {

set(errors, [field.id, ...Object.values(indexes || {})], fieldErrors.length ? fieldErrors : undefined);

this._emit('field.updated', update);

this._setState({
data: clone(data),
errors: clone(errors),
Expand Down Expand Up @@ -364,7 +372,13 @@ export class Form {
* @internal
*/
_getModules() {
return [ExpressionLanguageModule, MarkdownRendererModule, ViewerCommandsModule, RepeatRenderModule];
return [
ExpressionLanguageModule,
ExpressionFieldModule,
MarkdownRendererModule,
ViewerCommandsModule,
RepeatRenderModule,
];
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
export class ExpressionLoopPreventer {
constructor(eventBus) {
this._computedExpressions = [];

eventBus.on('field.updated', ({ doNotRecompute }) => {
if (doNotRecompute) {
return;
}

this.reset();
});

eventBus.on('import.done', this.reset.bind(this));
eventBus.on('reset', this.reset.bind(this));
}

requestOnce(expression) {
if (this._computedExpressions.includes(expression)) {
return false;
}

this._computedExpressions.push(expression);

return true;
}

reset() {
this._computedExpressions = [];
}
}

ExpressionLoopPreventer.$inject = ['eventBus'];
8 changes: 8 additions & 0 deletions packages/form-js-viewer/src/features/expressionField/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ExpressionLoopPreventer } from './ExpressionLoopPreventer';

export const ExpressionFieldModule = {
__init__: ['expressionLoopPreventer'],
expressionLoopPreventer: ['type', ExpressionLoopPreventer],
};

export { ExpressionLoopPreventer };
2 changes: 2 additions & 0 deletions packages/form-js-viewer/src/features/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
export { ExpressionLanguageModule } from './expressionLanguage';
export { ExpressionFieldModule } from './expressionField';
export { MarkdownRendererModule } from './markdown';
export { ViewerCommandsModule } from './viewerCommands';
export { RepeatRenderModule } from './repeatRender';

export * from './expressionLanguage';
export * from './expressionField';
export * from './markdown';
export * from './viewerCommands';
export * from './repeatRender';
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,18 @@ export function ExpressionField(props) {
const evaluation = useExpressionEvaluation(expression);
const evaluationMemo = useDeepCompareMemoize(evaluation);
const eventBus = useService('eventBus');
const expressionLoopPreventer = useService('expressionLoopPreventer');

const sendValue = useCallback(() => {
onChange && onChange({ field, value: evaluationMemo });
onChange && onChange({ field, value: evaluationMemo, doNotRecompute: true });
}, [field, evaluationMemo, onChange]);

useEffect(() => {
if (computeOn !== 'change' || isEqual(evaluationMemo, value)) {
if (computeOn !== 'change' || isEqual(evaluationMemo, value) || !expressionLoopPreventer.requestOnce(this)) {
return;
}
sendValue();
}, [computeOn, evaluationMemo, sendValue, value]);
});

useEffect(() => {
if (computeOn === 'presubmit') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('ExpressionField', function () {
});

// then
expect(onChangeSpy.calledWith({ field, value: 2 })).to.be.true;
expect(onChangeSpy.calledWith({ field, value: 2, doNotRecompute: true })).to.be.true;
});

it('should re-evaluate when the expression result changes', function () {
Expand Down Expand Up @@ -88,7 +88,7 @@ describe('ExpressionField', function () {
});

// then
expect(onChangeSpy.calledWith({ field, value: 3 })).to.be.true;
expect(onChangeSpy.calledWith({ field, value: 3, doNotRecompute: true })).to.be.true;
});

it('should not evaluate on intialization if computeOn presubmit', function () {
Expand Down Expand Up @@ -148,7 +148,7 @@ describe('ExpressionField', function () {
});

// then
expect(onChangeSpy.calledWith({ field, value: 2 })).to.be.true;
expect(onChangeSpy.calledWith({ field, value: 2, doNotRecompute: true })).to.be.true;
});
});

Expand Down

0 comments on commit 7565fc3

Please sign in to comment.