Skip to content

Conversation

jdegand
Copy link
Contributor

@jdegand jdegand commented Aug 21, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[X] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #4917
Closes #4918
Closes #4930

What is the new behavior?

Tests will run from the www folder (npx nx run www:test). There is a warning from tsconfig.spec.json file about missing types from vitest, but this doesn't prevent tests from running. I've tried many different changes and this never goes away. The npm packages got updated when I ran an analog command to regenerate the files.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

Copy link

netlify bot commented Aug 21, 2025

Deploy Preview for ngrx-io canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 5dfc2a8
🔍 Latest deploy log https://app.netlify.com/projects/ngrx-io/deploys/68c5b5dd95971d0008d9f65b

Copy link

netlify bot commented Aug 21, 2025

Deploy Preview for ngrx-site-v19 ready!

Name Link
🔨 Latest commit 5dfc2a8
🔍 Latest deploy log https://app.netlify.com/projects/ngrx-site-v19/deploys/68c5b5ddd4a85a0008550f16
😎 Deploy Preview https://deploy-preview-4934--ngrx-site-v19.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@jdegand
Copy link
Contributor Author

jdegand commented Aug 21, 2025

I just ran tests on main and the signals tests fail there as well. So that failure doesn't appear related to these code changes.

@markostanimirovic
Copy link
Member

@jdegand
Copy link
Contributor Author

jdegand commented Aug 22, 2025

@markostanimirovic The www tests fail because of #4930. I still don't get why the signals tests fail in this PR, but not in other ones. The signals tests timeout and maybe there is a potential memory leak.

@markostanimirovic
Copy link
Member

@markostanimirovic The www tests fail because of #4930. I still don't get why the signals tests fail in this PR, but not in other ones. The signals tests timeout and maybe there is a potential memory leak.

@jdegand Let's then fix #4930 first. I left a comment there.

@jdegand
Copy link
Contributor Author

jdegand commented Aug 22, 2025

@markostanimirovic I added the angular plugin, but I didn't put it ahead of the analog plugin. The www tests now pass.

@jdegand
Copy link
Contributor Author

jdegand commented Aug 24, 2025

The manualChunks change could be source of signals tests failure. Maybe, the parsing is too big and then those tests timeout.

@jdegand
Copy link
Contributor Author

jdegand commented Aug 26, 2025

Excluded examples folder from tests in www. Are these example projects going to be updated or a few removed?

The use of a constructor in place of an ngOnInit call may explain the injector error in app.component.ts.

@timdeschryver
Copy link
Member

Excluded examples folder from tests in www. Are these example projects going to be updated or a few removed?

The use of a constructor in place of an ngOnInit call may explain the injector error in app.component.ts.

The examples are referenced within our docs, or offered as a Stackblitz project.
We can go over them to see if the spec files are needed or can be removed.
For now I think it's good to exclude them from the tests.

@jdegand
Copy link
Contributor Author

jdegand commented Aug 30, 2025

Need to further investigate issues with angular-vitest-plugin. There are a few issues that are unresolved with Vitest's latest changes:

  resolve: {
    conditions: [...defaultClientConditions],
  },

is a workaround.

I tried using a zoneless vite.config.ts in www from the analog docs. It didn't seem to help.

I have started looking at more files for issues. There definitely appears to be a misconfiguration somewhere.

The signals tests always timeout. The timeout failures are all related to zone.js. An import issue in a test-setup?

Update: cacheDir scope could be important. All tests pull from same cache so race conditions and flakiness.

@jdegand
Copy link
Contributor Author

jdegand commented Sep 12, 2025

There was an error in the configuration for www and I used inlineStyles inside angular vs analog. The newer version of the plugin doesn't require the vite object key.

Changing Object.assign and using globalThis seemed to help the signals tests although there is still one test timing out.

Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Thanks @jdegand , I pushed some changes to push this over the finish line.

  • force push (no changes) was to trigger the build with our CI activated
  • this detected failing tests on all modules because the missing ts-node dependency (I don't know how I missed that in my cleanup) => I re-added the dependency
  • the signals were still giving a time-out. I noticed this were cases where there were multiple expectSnippets => I refactored these cases to only compile the snippet once

@jdegand
Copy link
Contributor Author

jdegand commented Sep 13, 2025

@timdeschryver The www tests still can fail (inconsistent).

Below is an app.component change that makes the tests pass:

import {
  Component,
  Injector,
  PLATFORM_ID,
  inject,
  OnDestroy,
} from '@angular/core';
import { RouterOutlet } from '@angular/router';
import { isPlatformBrowser } from '@angular/common';
import { MenuComponent } from './components/menu.component';
import { MarkdownSymbolLinkComponent } from './components/docs/markdown-symbol-link.component';
import { AlertComponent } from './components/docs/alert.component';
import { CodeExampleComponent } from './components/docs/code-example.component';
import { CodeTabsComponent } from './components/docs/code-tabs.component';
import { StackblitzComponent } from './components/docs/stackblitz.component';
import { FooterComponent } from './components/footer.component';

@Component({
  selector: 'ngrx-root',
  imports: [
    RouterOutlet,
    MenuComponent,
    MarkdownSymbolLinkComponent,
    AlertComponent,
    CodeExampleComponent,
    StackblitzComponent,
    FooterComponent,
  ],
  template: `
    <ngrx-menu></ngrx-menu>
    <router-outlet></router-outlet>
    <ngrx-footer></ngrx-footer>
  `,
  styles: [
    `
      :host {
        display: block;
        position: relative;
        width: calc(100% - 270px);
        left: 270px;
        @media only screen and (max-width: 1280px) {
          width: 100%;
          left: 0;
        }
      }

      ngrx-menu {
        position: fixed;
        top: 0;
        left: 0;
      }
    `,
  ],
})
export class AppComponent implements OnDestroy {
  private isAlive = true;
  private injector = inject(Injector);
  private platformId = inject(PLATFORM_ID);

  constructor() {
    if (isPlatformBrowser(this.platformId)) {
      this.installCustomElements();
    }
  }

  ngOnDestroy(): void {
    this.isAlive = false;
  }

  async installCustomElements() {
    if (!this.isAlive) return;

    const { createCustomElement } = await import('@angular/elements');

    if (!this.isAlive) return;

    const symbolLinkElement = createCustomElement(MarkdownSymbolLinkComponent, {
      injector: this.injector,
    });
    customElements.define('ngrx-docs-symbol-link', symbolLinkElement);

    if (!this.isAlive) return;

    const alertElement = createCustomElement(AlertComponent, {
      injector: this.injector,
    });
    customElements.define('ngrx-docs-alert', alertElement);

    if (!this.isAlive) return;

    const codeExampleElement = createCustomElement(CodeExampleComponent, {
      injector: this.injector,
    });
    customElements.define('ngrx-code-example', codeExampleElement);

    if (!this.isAlive) return;

    const codeTabsElement = createCustomElement(CodeTabsComponent, {
      injector: this.injector,
    });
    customElements.define('ngrx-code-tabs', codeTabsElement);

    if (!this.isAlive) return;

    const stackblitzElement = createCustomElement(StackblitzComponent, {
      injector: this.injector,
    });
    customElements.define('ngrx-docs-stackblitz', stackblitzElement);
  }
}

@jdegand
Copy link
Contributor Author

jdegand commented Sep 13, 2025

@timdeschryver I updated the app component and changed the test-setup. I ran the www tests many times and they never failed. I think this should be it although the analog plugin API is tweaked in the next version so the vite config will need a change when that dependency is updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants