Make HTML ID generation fast

Replaces HTML ID generation with a simple monotonic sequence to avoid several expensive hash lookups for every generated ID. This entirely eliminates the chance for collisions under regular circumstances, and the default seed even performs better in some adverse circumstances, like multiple instances of the same library sharing an ID namespace on the same page. The only potential downside is that IDs might not be particularly semantically helpful, but that can be mitigated by just appending that information instead. This might be a breaking change if this module is considered a public API.
eclipsesource · Oct 24, 2024 · e12bd5f · e12bd5f
1 parent 08bb7df
commit e12bd5f
Show file tree

Hide file tree

Showing 23 changed files with 95 additions and 193 deletions.
diff --git a/packages/angular/src/library/abstract-control.ts b/packages/angular/src/library/abstract-control.ts
@@ -29,7 +29,6 @@ import {
   JsonFormsState,
   JsonSchema,
   OwnPropsOfControl,
-  removeId,
   StatePropsOfControl,
 } from '@jsonforms/core';
 import { Component, Input, OnDestroy, OnInit } from '@angular/core';
@@ -149,7 +148,6 @@ export abstract class JsonFormsAbstractControl<
 
   ngOnDestroy() {
     super.ngOnDestroy();
-    removeId(this.id);
   }
 
   isEnabled(): boolean {

diff --git a/packages/angular/src/library/jsonforms.component.ts b/packages/angular/src/library/jsonforms.component.ts
@@ -32,7 +32,7 @@ import {
   ViewContainerRef,
 } from '@angular/core';
 import {
-  createId,
+  nextId,
   isControl,
   getConfig,
   JsonFormsProps,
@@ -143,7 +143,7 @@ export class JsonFormsOutlet
         const controlInstance = instance as JsonFormsControl;
         if (controlInstance.id === undefined) {
           const id = isControl(props.uischema)
-            ? createId(props.uischema.scope)
+            ? nextId() + props.uischema.scope
             : undefined;
           (instance as JsonFormsControl).id = id;
         }

diff --git a/packages/core/src/util/ids.ts b/packages/core/src/util/ids.ts
@@ -23,30 +23,21 @@
   THE SOFTWARE.
 */
 
-const usedIds: Set<string> = new Set<string>();
+let idNamespace: string;
+let idIndex: number;
 
-const makeId = (idBase: string, iteration: number) =>
-  iteration <= 1 ? idBase : idBase + iteration.toString();
-
-const isUniqueId = (idBase: string, iteration: number) => {
-  const newID = makeId(idBase, iteration);
-  return !usedIds.has(newID);
-};
-
-export const createId = (proposedId: string) => {
-  if (proposedId === undefined) {
-    // failsafe to avoid endless loops in error cases
-    proposedId = 'undefined';
-  }
-  let tries = 0;
-  while (!isUniqueId(proposedId, tries)) {
-    tries++;
-  }
-  const newID = makeId(proposedId, tries);
-  usedIds.add(newID);
-  return newID;
+export const seedIds = (namespace = 'jsonforms', index = 0) => {
+  idNamespace = namespace;
+  idIndex = index;
 };
 
-export const removeId = (id: string) => usedIds.delete(id);
+// This has a salt in the unlikely case that someone bundled multiple instances
+// of this module on a single site.
+seedIds(`jsonforms${Math.random().toString(32).slice(2)}`);
 
-export const clearAllIds = () => usedIds.clear();
+/**
+ * Generate an ID that's unique within the current execution context. This is
+ * intended for HTML ID generation. Does not guarantee stability across SSR
+ * boundaries!
+ */
+export const nextId = () => `:${idNamespace}:${(idIndex++).toString(36)}:`;
diff --git a/packages/core/test/mappers/cell.test.ts b/packages/core/test/mappers/cell.test.ts
@@ -25,7 +25,7 @@
 import test from 'ava';
 import * as _ from 'lodash';
 import * as Redux from 'redux';
-import { clearAllIds, createAjv, validate } from '../../src/util';
+import { seedIds, createAjv, validate } from '../../src/util';
 import { UPDATE_DATA, UpdateAction } from '../../src/actions';
 import configureStore from 'redux-mock-store';
 import { JsonFormsState } from '../../src/store';
@@ -263,7 +263,7 @@ test('mapStateToCellProps - data', (t) => {
 });
 
 test('mapStateToCellProps - id', (t) => {
-  clearAllIds();
+  seedIds();
   const ownProps = {
     uischema: coreUISchema,
     id: '#/properties/firstName',

diff --git a/packages/core/test/mappers/renderer.test.ts b/packages/core/test/mappers/renderer.test.ts
@@ -64,7 +64,7 @@ import {
   mapStateToOneOfEnumControlProps,
   mapStateToOneOfProps,
 } from '../../src/mappers';
-import { clearAllIds, convertDateToString, createAjv } from '../../src/util';
+import { seedIds, convertDateToString, createAjv } from '../../src/util';
 import { rankWith } from '../../src';
 
 const middlewares: Redux.Middleware[] = [];
@@ -429,7 +429,7 @@ test('mapStateToControlProps - no duplicate error messages', (t) => {
 });
 
 test('mapStateToControlProps - id', (t) => {
-  clearAllIds();
+  seedIds();
   const ownProps = {
     uischema: coreUISchema,
     id: '#/properties/firstName',

diff --git a/packages/material-renderers/src/layouts/ExpandPanelRenderer.tsx b/packages/material-renderers/src/layouts/ExpandPanelRenderer.tsx
@@ -5,8 +5,6 @@ import React, {
   Fragment,
   ReducerAction,
   useMemo,
-  useState,
-  useEffect,
   useCallback,
 } from 'react';
 import {
@@ -25,8 +23,7 @@ import {
   update,
   JsonFormsCellRendererRegistryEntry,
   JsonFormsUISchemaRegistryEntry,
-  createId,
-  removeId,
+  nextId,
   ArrayTranslations,
   computeChildLabel,
   UpdateArrayContext,
@@ -88,13 +85,8 @@ export interface ExpandPanelProps
     DispatchPropsOfExpandPanel {}
 
 const ExpandPanelRendererComponent = (props: ExpandPanelProps) => {
-  const [labelHtmlId] = useState<string>(createId('expand-panel'));
-
-  useEffect(() => {
-    return () => {
-      removeId(labelHtmlId);
-    };
-  }, [labelHtmlId]);
+  // TODO: Should probably use React.useId() when support for React < 18 is dropped.
+  const labelHtmlId = useMemo(() => nextId() + 'expand-panel', []);
 
   const {
     enabled,

diff --git a/packages/react/src/JsonForms.tsx b/packages/react/src/JsonForms.tsx
@@ -28,9 +28,8 @@ import type Ajv from 'ajv';
 import type { ErrorObject } from 'ajv';
 import { UnknownRenderer } from './UnknownRenderer';
 import {
-  createId,
+  nextId,
   Generate,
-  isControl,
   JsonFormsCellRendererRegistryEntry,
   JsonFormsCore,
   JsonFormsI18nState,
@@ -40,7 +39,6 @@ import {
   JsonSchema,
   Middleware,
   OwnPropsOfJsonFormsRenderer,
-  removeId,
   UISchemaElement,
   ValidationMode,
 } from '@jsonforms/core';
@@ -49,129 +47,57 @@ import {
   withJsonFormsRendererProps,
 } from './JsonFormsContext';
 
-interface JsonFormsRendererState {
-  id: string;
-}
-
 export interface JsonFormsReactProps {
   onChange?(state: Pick<JsonFormsCore, 'data' | 'errors'>): void;
   middleware?: Middleware;
 }
 
-export class JsonFormsDispatchRenderer extends React.Component<
-  JsonFormsProps,
-  JsonFormsRendererState
-> {
-  constructor(props: JsonFormsProps) {
-    super(props);
-    this.state = {
-      id: isControl(props.uischema)
-        ? createId(props.uischema.scope)
-        : undefined,
-    };
-  }
-
-  componentWillUnmount() {
-    if (isControl(this.props.uischema)) {
-      removeId(this.state.id);
-    }
-  }
-
-  componentDidUpdate(prevProps: JsonFormsProps) {
-    if (prevProps.schema !== this.props.schema) {
-      removeId(this.state.id);
-      this.setState({
-        id: isControl(this.props.uischema)
-          ? createId(this.props.uischema.scope)
-          : undefined,
-      });
-    }
-  }
-
-  render() {
-    const {
-      schema,
-      rootSchema,
-      uischema,
-      path,
-      enabled,
-      renderers,
-      cells,
-      config,
-    } = this.props as JsonFormsProps;
-
-    return (
-      <TestAndRender
-        uischema={uischema}
-        schema={schema}
-        rootSchema={rootSchema}
-        path={path}
-        enabled={enabled}
-        renderers={renderers}
-        cells={cells}
-        id={this.state.id}
-        config={config}
-      />
+export const JsonFormsDispatchRenderer = React.memo(
+  function JsonFormsDispatchRenderer(props: JsonFormsProps) {
+    // TODO: Should probably use React.useId() when support for React < 18 is dropped.
+    const id = useMemo(nextId, [props.schema]);
+    const testerContext = useMemo(
+      () => ({
+        rootSchema: props.rootSchema,
+        config: props.config,
+      }),
+      [props.rootSchema, props.config]
     );
-  }
-}
-
-const TestAndRender = React.memo(function TestAndRender(props: {
-  uischema: UISchemaElement;
-  schema: JsonSchema;
-  rootSchema: JsonSchema;
-  path: string;
-  enabled: boolean;
-  renderers: JsonFormsRendererRegistryEntry[];
-  cells: JsonFormsCellRendererRegistryEntry[];
-  id: string;
-  config: any;
-}) {
-  const testerContext = useMemo(
-    () => ({
-      rootSchema: props.rootSchema,
-      config: props.config,
-    }),
-    [props.rootSchema, props.config]
-  );
-  const renderer = useMemo(
-    () =>
-      maxBy(props.renderers, (r) =>
-        r.tester(props.uischema, props.schema, testerContext)
-      ),
-    [props.renderers, props.uischema, props.schema, testerContext]
-  );
-  if (
-    renderer === undefined ||
-    renderer.tester(props.uischema, props.schema, testerContext) === -1
-  ) {
-    return <UnknownRenderer type={'renderer'} />;
-  } else {
-    const Render = renderer.renderer;
-    return (
-      <Render
-        uischema={props.uischema}
-        schema={props.schema}
-        path={props.path}
-        enabled={props.enabled}
-        renderers={props.renderers}
-        cells={props.cells}
-        id={props.id}
-      />
+    const renderer = useMemo(
+      () =>
+        maxBy(props.renderers, (r) =>
+          r.tester(props.uischema, props.schema, testerContext)
+        ),
+      [props.renderers, props.uischema, props.schema, testerContext]
     );
+    if (
+      renderer === undefined ||
+      renderer.tester(props.uischema, props.schema, testerContext) === -1
+    ) {
+      return <UnknownRenderer type={'renderer'} />;
+    } else {
+      const Render = renderer.renderer;
+      return (
+        <Render
+          uischema={props.uischema}
+          schema={props.schema}
+          path={props.path}
+          enabled={props.enabled}
+          renderers={props.renderers}
+          cells={props.cells}
+          id={id}
+        />
+      );
+    }
   }
-});
+);
 
 /**
  * @deprecated Since Version 3.0 this optimization renderer is no longer necessary.
  * Use `JsonFormsDispatch` instead.
  * We still export it for backward compatibility
  */
-export class ResolvedJsonFormsDispatchRenderer extends JsonFormsDispatchRenderer {
-  constructor(props: JsonFormsProps) {
-    super(props);
-  }
-}
+export const ResolvedJsonFormsDispatchRenderer = JsonFormsDispatchRenderer;
 
 export const JsonFormsDispatch: ComponentType<OwnPropsOfJsonFormsRenderer> =
   withJsonFormsRendererProps(JsonFormsDispatchRenderer);

diff --git a/packages/vue-vuetify/tests/unit/additional/LabelRenderer.spec.ts b/packages/vue-vuetify/tests/unit/additional/LabelRenderer.spec.ts
@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach } from 'vitest';
-import { clearAllIds } from '@jsonforms/core';
+import { seedIds } from '@jsonforms/core';
 import LabelRenderer from '../../../src/additional/LabelRenderer.vue';
 import { entry as labelRendererEntry } from '../../../src/additional/LabelRenderer.entry';
 import { mountJsonForms } from '../util';
@@ -20,7 +20,7 @@ describe('LabelRenderer.vue', () => {
 
   beforeEach(() => {
     // clear all ids to guarantee that the snapshots will always be generated with the same ids
-    clearAllIds();
+    seedIds();
     wrapper = mountJsonForms(data, schema, renderers, uischema);
   });
 

diff --git a/packages/vue-vuetify/tests/unit/additional/ListWithDetailRenderer.spec.ts b/packages/vue-vuetify/tests/unit/additional/ListWithDetailRenderer.spec.ts
@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach } from 'vitest';
-import { clearAllIds, type Translator } from '@jsonforms/core';
+import { seedIds, type Translator } from '@jsonforms/core';
 import ListWithDetailRenderer from '../../../src/additional/ListWithDetailRenderer.vue';
 import { entry as listWithDetailRendererEntry } from '../../../src/additional/ListWithDetailRenderer.entry';
 import { mountJsonForms } from '../util';
@@ -28,7 +28,7 @@ describe('ListWithDetailRenderer.vue', () => {
 
   beforeEach(() => {
     // clear all ids to guarantee that the snapshots will always be generated with the same ids
-    clearAllIds();
+    seedIds();
     wrapper = mountJsonForms(data, schema, renderers, uischema, undefined, {
       translate: ((id, defaultMessage) => {
         if (id.endsWith('addAriaLabel')) {

diff --git a/packages/vue-vuetify/tests/unit/complex/ArrayControlRenderer.spec.ts b/packages/vue-vuetify/tests/unit/complex/ArrayControlRenderer.spec.ts
@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach } from 'vitest';
-import { clearAllIds, type Translator } from '@jsonforms/core';
+import { seedIds, type Translator } from '@jsonforms/core';
 import ArrayControlRenderer from '../../../src/complex/ArrayControlRenderer.vue';
 import { entry as arrayControlRendererEntry } from '../../../src/complex/ArrayControlRenderer.entry';
 import { mountJsonForms } from '../util';
@@ -27,7 +27,7 @@ describe('ArrayControlRenderer.vue', () => {
 
   beforeEach(() => {
     // clear all ids to guarantee that the snapshots will always be generated with the same ids
-    clearAllIds();
+    seedIds();
     wrapper = mountJsonForms(data, schema, renderers, uischema, undefined, {
       translate: ((id, defaultMessage) => {
         if (id.endsWith('addAriaLabel')) {

diff --git a/packages/vue-vuetify/tests/unit/complex/OneOfRenderer.spec.ts b/packages/vue-vuetify/tests/unit/complex/OneOfRenderer.spec.ts
@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach } from 'vitest';
-import { clearAllIds, type Translator } from '@jsonforms/core';
+import { seedIds, type Translator } from '@jsonforms/core';
 import OneOfControlRenderer from '../../../src/complex/OneOfRenderer.vue';
 import { entry as oneOfControlRendererEntry } from '../../../src/complex/OneOfRenderer.entry';
 import { mountJsonForms } from '../util';
@@ -38,7 +38,7 @@ describe('OneOfRenderer.vue', () => {
 
   beforeEach(() => {
     // clear all ids to guarantee that the snapshots will always be generated with the same ids
-    clearAllIds();
+    seedIds();
     wrapper = mountJsonForms(data, schema, renderers, uischema, undefined, {
       translate: ((id, defaultMessage) => {
         if (id.endsWith('clearDialogAccept')) {