Skip to content

allow using lazy loadable renderers in angular #2446

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

allow using lazy loadable renderers in angular #2446

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4c58e98
allow using lazy loadable renderers
dsl400 May 11, 2025
13a6223
bootstrap forms builder
dsl400 May 11, 2025
22c2af3
repo config
dsl400 May 11, 2025
3f5b96f
repository configuration for angular packages dev mode
dsl400 May 11, 2025
0db28a5
Merge branch 'master' of https://github.com/dsl400/jsonforms
dsl400 May 11, 2025
bd4f68a
Revert "Merge branch 'master' of https://github.com/dsl400/jsonforms"
dsl400 May 11, 2025
a32e454
Revert "repository configuration for angular packages dev mode"
dsl400 May 11, 2025
5e32b15
allow using lazy loadable renderers
dsl400 May 11, 2025
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
24 changes: 23 additions & 1 deletion packages/angular/src/library/jsonforms.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/
import maxBy from 'lodash/maxBy';
import {
ComponentFactory,
ComponentFactoryResolver,
Directive,
Input,
Expand Down Expand Up @@ -126,11 +127,32 @@ export class JsonFormsOutlet
renderer !== undefined &&
renderer.tester(uischema, schema, testerContext) !== -1
) {
bestComponent = renderer.renderer;
if (renderer.renderer instanceof Promise) {
renderer.renderer.then((resolvedRenderer) => {
Copy link
Member

Choose a reason for hiding this comment

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

There can be a race condition here:

  1. The outlet is invoked, however the renderer (Renderer A) is a promise and needs some time to complete
  2. There is an update in the mean time, another renderer (Renderer B) is required and it is awaited too
  3. The Renderer B resolves first and renders
  4. Now finally Renderer A resolves and will be used for rendering, although Renderer B is the one which should be used.

Copy link
Member

Choose a reason for hiding this comment

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

We could also think about storing the resolved component somewhere, maybe even in the registry entry. This would be useful so that once a renderer is resolved, we avoid the whole "LoadingSpinner" code path the next time it is invoked.

bestComponent = resolvedRenderer;
Copy link
Member

Choose a reason for hiding this comment

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

This assignment points to a code smell:

  • Either it has no effect, because the update code is already completed,
  • Or the promise is already resolved and the Javascript engine decides to also run this "async" snippet immediately. However in that case the component will be rendered twice, once by this "async" snipped and once by the update code.

const componentFactory =
this.componentFactoryResolver.resolveComponentFactory(
bestComponent
);
this.renderComponent(componentFactory, uischema, schema, props);
});
return;
} else {
bestComponent = renderer.renderer;
}
}

const componentFactory =
this.componentFactoryResolver.resolveComponentFactory(bestComponent);
this.renderComponent(componentFactory, uischema, schema, props);
Comment on lines 145 to +147
Copy link
Member

Choose a reason for hiding this comment

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

For async components, this will lead to the UnknownRenderer being rendered first which is misleading and might look like an error to the user. Instead we should render a LoadingRenderer or something similar.

}

private renderComponent(
componentFactory: ComponentFactory<any>,
uischema: UISchemaElement,
schema: JsonSchema,
props: JsonFormsProps
) {
this.viewContainerRef.clear();
const currentComponentRef =
this.viewContainerRef.createComponent(componentFactory);
Expand Down