[Bug]: Storybook destroys injector when using createCustomElement and document.createElement

[fullstackdeveloperbilbao]

Describe the bug

When creating a custom element, then switching to another story and creating another custom element, storybook destroys the injector and angular throws an error.

To Reproduce

Clone repository from:

https://github.com/fullstackdeveloperbilbao/storybook-injector-error.git

After cloning run:

• npm install
• npm run storybook
• On FIRST story click Add item button.
• Switch to SECOND story.
• On SECOND story click Add item button.

The error should be displayed.

System

System:
    OS: macOS 14.2
    CPU: (8) arm64 Apple M1 Pro
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 16.19.0 - ~/.nvm/versions/node/v16.19.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.19.0/bin/yarn
    npm: 8.19.3 - ~/.nvm/versions/node/v16.19.0/bin/npm <----- active
  Browsers:
    Chrome: 120.0.6099.109
    Safari: 17.2
  npmPackages:
    @storybook/addon-essentials: ^7.6.4 => 7.6.4 
    @storybook/addon-interactions: ^7.6.4 => 7.6.4 
    @storybook/addon-links: ^7.6.4 => 7.6.4 
    @storybook/angular: ^7.6.4 => 7.6.4 
    @storybook/blocks: ^7.6.4 => 7.6.4 
    @storybook/test: ^7.6.4 => 7.6.4 
    storybook: ^7.6.4 => 7.6.4

Additional context

No response

[Marklb]

I think I know what is happening, but I am not experienced enough with Angular elements to know how the problem should be fixed, yet.

Below, I will provide a solution, but it may not be a good solution and most likely is a minor memory leak, My solution should work though, if you just need to get it working.

What I think is the problem:

Between stories, Storybook tries to clear any knowledge of the previous story's Angular app. The CustomElementRegistry is a browser feature, not something in Angular's compiler or existing in the Angular injector. So, if the page isn't refreshed, between stories, and the same tag is used, the registry is going to still have the first story's registered component, even though we already destroyed the application that it was registered for.

I don't know if there is a way to clear or reset the browser's CustomElementRegistry. It looks like the registry has an upgrade function that, I assume, could replace the registered component, but I don't expect user's code to care about that, just so they can be compatible with Storybook. Maybe Storybook can proxy the CustomElementRegistry and handle when elements should be considered unregistered then upgrade them as needed. I am just thinking, but I will see if I can find some projects that unit test custom elements and see how they deal with this.

Possible solution/workaround:

This may not be a good solution, but it does work in this scenario, until an actual solution is decided.

First, this may not have anything to do with your problem, but I just noticed it as I was briefly looking through the elements code. Isn't your code breaking this recommendation that Angular points out as important? https://github.com/angular/angular/blob/d7e7409e8115210614ff69f6bc6ddae1a5076986/adev/src/content/guide/elements.md?plain=1#L52-L54

Thanks for the simple clear repro. I moved your code into a Stackblitz, to share it easier with my workaround. Original code, mostly from original repro: https://stackblitz.com/edit/github-ri7ynh My workaround: https://stackblitz.com/edit/github-ri7ynh-x17jev

What I did: In your .storybook/preview.ts, I provided a unique value in applicationConfig, which is just an incrementing number in my quick example.

import { APP_IDENTIFIER } from '../src/app/injector-error/injector-error.component';

let _idx = 0;

const preview: Preview = {
  decorators: [
    applicationConfig({
      providers: [
        { provide: APP_IDENTIFIER, useFactory: () => `${_idx++}` },
      ],
    }),
...

Then in your injector-error.component.ts I appended that APP_IDENTIFIER to the registered element's tag.

export const APP_IDENTIFIER = new InjectionToken<string>('APP_IDENTIFIER');
...
@Component({ ... })
export class InjectorErrorComponent {
  private readonly _identifier = inject(APP_IDENTIFIER);
  ...
  private defineListItemElement() {
    if (isNil(customElements.get(`app-list-item--${this._identifier}`))) {
      const validationElement = createCustomElement(ListItemComponent, {
        injector: this._inj,
      });
      customElements.define(`app-list-item--${this._identifier}`, validationElement);
    }
  }

  private createListItemElement() {
    const validationEl: NgElement & WithProperties<ListItemComponent> =
      // eslint-disable-next-line @typescript-eslint/no-explicit-any
      document.createElement(`app-list-item--${this._identifier}`) as any;

    return validationEl;
  }
  ...
}

I really don't like the application needing to do anything specifically for Storybook, but in your app you could probably provide { provide: APP_IDENTIFIER, useFactory: () => '' } and not affect the actual output of your app.

[Marklb]

I was thinking about this some more and I don't think my initial idea of potentially proxying the CustomElementRegistry and document.createElement would really fix the problem, if it would even work. Maybe as a fix for the CanvasRenderer, but the DocsRenderer wouldn't be fixed, unless inline rendering isn't used. With inline rendering, multiple apps rendered at the same time would not be possible, without each one using a unique tag, because document.createElement isn't guaranteed to be called from a place that identifies which Angular app is calling it, so swapping out the defined custom element wouldn't be reliable.

Another potential solution could be an option to force a page refresh, when doing a full render, and not using inline rendering. That may be the most reliable solution, but I don't like the idea of so many refreshes.

Maybe someone more familiar with web-components would have an idea, but that is my thoughts so far.

[fullstackdeveloperbilbao]

Hi,

Based on valid custom element naming HTML standards, I have created a simple monkey patching to attach the appIdentifier to customElements.get, customElements.define, customElements.whenDefined and document.createElement.

As long as this allows me to continue working I will use it.

private monkeyPatchElementsAPI(appIdentifier: string) {
    const originalGet = customElements.get.bind(customElements);
    customElements.get = function (name) {
      return originalGet(`${name}-${appIdentifier}`);
    };

    const originalDefine = customElements.define.bind(customElements);
    customElements.define = function (name, constructor, options) {
      originalDefine(`${name}-${appIdentifier}`, constructor, options);
    };

    const originalWhenDefined = customElements.whenDefined.bind(customElements);
    customElements.whenDefined = function (name: string) {
      return originalWhenDefined(`${name}-${appIdentifier}`);
    };

    const originalCreateElement = document.createElement.bind(document);
    document.createElement = function (tagName: any, options: any) {
      // https://html.spec.whatwg.org/multipage/custom-elements.html#valid-custom-element-name
      const isValidCustomElementName = (name: string) => {
        // Blacklist of invalid names
        const invalidNames = [
          'annotation-xml',
          'color-profile',
          'font-face',
          'font-face-src',
          'font-face-uri',
          'font-face-format',
          'font-face-name',
          'missing-glyph',
        ];
        return !invalidNames.includes(name) && name.includes('-');
      };

      const isStoryBookElement = (name: string) => {
        return tagName === 'storybook-root';
      };

      if (isValidCustomElementName(tagName) && !isStoryBookElement(tagName)) {
        const element = originalCreateElement(
          `${tagName}-${appIdentifier}`,
          options
        );
        return element;
      } else {
        return originalCreateElement(`${tagName}`, options);
      }
    };
  }

As a note, this error comes from this code and as explained in line 114 The configuration's injector is the initial injector set on the class, and used by default for each created instance.

Another option would be to monkeypatch the createCustomElement API.

class NgElementImpl extends NgElement {
    // ... existing code ...

    private strategyFactory: ComponentNgElementStrategyFactory;

    private initStrategyFactory() {
      this.strategyFactory = new ComponentNgElementStrategyFactory(component, this.injector || config.injector);
    }

    setNewInjector(newInjector: Injector): void {
      this.injector = newInjector;
      this.initStrategyFactory();
      this._ngElementStrategy = null;
    }

    protected override get ngElementStrategy(): NgElementStrategy {
      if (!this._ngElementStrategy) {
        this._ngElementStrategy = this.strategyFactory.create(this.injector || config.injector);

        // ... existing code  ...
      }

      return this._ngElementStrategy!;
    }

    // ... existing code ...
  }

[Marklb]

@fullstackdeveloperbilbao That monkeyPatch looks sort of like what I was talking about doing, when I mentioned potentially proxying the CustomElementsRegistry. I don't know where you are placing it, but that will probably work as long as you disable inline rendering.

One problem with using it, with inline rendering is the following:

1. Open a Docs page, containing two inline rendered stories, which should be how Docs works by default.
2. Story one renders, with monkeyPatchElementsAPI('one').
3. Story two renders, with monkeyPatchElementsAPI('two').
4. Story one triggers an action that calls document.createElement('app-list-item'). Incorrect result, but may not throw an error that makes the problem obvious.
   • You may not get an error, because both story's Angular app are still rendered, but you most recently patched with 'two', so story one will create an element that is for the second story.

That scenario may not be a problem, as long as you know to disable inline rendering, but I am trying to find a solution that doesn't break inline rendering or can be made clear that it needs to be disabled for stories that use custom elements.

The only way I can think to throw a warning/error that makes it clear that unexpected side-effects may occur is to patch the CustomElementsRegistry methods with a warning, if multiple stories are rendered. That could give false positives, since an addon may be using the CustomElementsRegistry.

---

Even though I don't like monkeyPatching, if we could patch the createCustomElement function then that could be a way to reduce the false positives for when to warn user's about not using inline rendering. I don't think it would fix the problem that I mentioned, though.

---

I don't know where you call monkeyPatchElementsAPI, but it seems to affect Storybook and Angular, because Storybook and probably Angular use document.createElement. Maybe with some sort of heuristic that avoids it being used for the wrong elements could avoid that, but I am not sure.

I did play with your monkeyPatchElementsAPI function, by finding somewhere to call it and fix a few small bugs. The following Stackblitz is where I did that. It seems to work, using the canvas renderer, but not on the Docs. I didn't try disabling inline rendering, since I would would want it to be obviously pointed out to the user that something is wrong in whatever fix is done. If you click "Add Item" on two stories on the Docs page, you will see that their list contains the same custom element, instead of the one it defined. Here is that Stackblitz: https://stackblitz.com/edit/github-ri7ynh-89muvn

[fullstackdeveloperbilbao]

To avoid creating unnecessary custom elements when docs are selected I have added a check to the preview decorator context that increases the _idx only if the user is not on docs, or if the user is on docs but it is the first story.

let _idx = 0;
let isDocsIdentifier = false;
...

const preview: Preview = {
  decorators: [
    ...
    (storyFunc, context) => {
          if (context.viewMode === 'docs') {
            if (!isDocsIdentifier) {
              _idx++;
              isDocsIdentifier = true;
            }
          } else {
            isDocsIdentifier = false;
            _idx++;
          }
          monkeyPatchElementsAPI(`${_idx}`);
          return storyFunc(context);
        }
  ]