Skip to content
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.

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

Update to ng19 and set standalone property for all components #683

Merged
merged 11 commits into from
Nov 27, 2024

Conversation

johncrim
Copy link
Contributor

@johncrim johncrim commented Nov 25, 2024

Fixes: #682

PR Checklist

Please check if your PR fulfills the following requirements:

Existing tests cover the case being fixed.

PR Type

What kind of change does this PR introduce?

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

Updates build dependencies to ng 19, which has a breaking change:

What is the current behavior?

After upgrading to ng 19, any use of SpectatorHost, or any other root component that uses HostComponent results in this error:

Failed: Unexpected "HostComponent" found in the "declarations" array of the "TestBed.configureTestingModule" call, "HostComponent" is marked as standalone and can't be declared in any NgModule - did you intend to import it instead (by adding it to the "imports" array)?

Issue Number: #682

What is the new behavior?

The HostComponent is now marked with standalone: false. All the other changes were to make the build and tests work with ng 19.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

There are no breaking changes - I tested locally with hundreds of jest and karma tests, using both ng 18 and ng 19, and both worked as expected. No changes to tests required to fix the issue.

Other information

I also have a branch where I installed the latest version of yarn:
https://github.com/johncrim/spectator/tree/chore/ng19-update

I needed current yarn to use yarn dlx for ng update. This branch includes all the changes in that branch, minus the changes to upgrade yarn. Let me know if you want the yarn upgrade or not.

For ng < 19, we need to keep `standalone: true`.
For ng >= 19, we need `standalone: false` on components, directives, and pipes.
```ts
const mock: any = { ...template } || {};
```
causes a compiler error.
Copy link

stackblitz bot commented Nov 25, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@johncrim
Copy link
Contributor Author

There's an issue with these changes, in that the Jest Matchers aren't working. I'm investigating.

But otherwise this works as expected in our test projects.

Matchers were broken by typescript upgrade:

As of TypeScript 5.5, the compiler no longer emits triple-slash directives to
the output files by default. However, there is a way to preserve these
directives in specific cases:

Use `preserve="true"` attribute in your triple-slash directive.
@johncrim johncrim force-pushed the chore/ng19-update-wout-yarn branch from e5bded5 to 94721d8 Compare November 26, 2024 00:54
@johncrim
Copy link
Contributor Author

I have fixed the broken matcher issue, and verified locally that these fixes run with our tests using ng19 and using ng18.

@NetanelBasal - please take a look when you have a moment.

@NetanelBasal
Copy link
Member

@johncrim thanks. Are there any breaking changes?

CHANGELOG.md Outdated

## v.Next
Copy link
Member

Choose a reason for hiding this comment

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

please revert, it should be updated automatically upon release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

@johncrim
Copy link
Contributor Author

johncrim commented Nov 26, 2024

Hi @NetanelBasal - I reverted the Changelog line.

There are no breaking changes* - I tested locally with hundreds of jest and karma tests, using both ng 18 and ng 19, and both worked as expected. No project changes needed for the upgrade. Asterisk explained next:

The one possible breaking change I noted is that the build output now only contains the /fesm2022 directory - it no longer includes the /esm2022 directory. This is apparently a result of changes in ng-packagr 19.

So the old dist package.json had:

  "exports": {
    "./package.json": {
      "default": "./package.json"
    },
    ".": {
      "types": "./index.d.ts",
      "esm2022": "./esm2022/ngneat-spectator.mjs",
      "esm": "./esm2022/ngneat-spectator.mjs",
      "default": "./fesm2022/ngneat-spectator.mjs"
    },
    "./internals": {
      "types": "./internals/index.d.ts",
      "esm2022": "./esm2022/internals/ngneat-spectator-internals.mjs",
      "esm": "./esm2022/internals/ngneat-spectator-internals.mjs",
      "default": "./fesm2022/ngneat-spectator-internals.mjs"
    },
    "./jest": {
      "types": "./jest/index.d.ts",
      "esm2022": "./esm2022/jest/ngneat-spectator-jest.mjs",
      "esm": "./esm2022/jest/ngneat-spectator-jest.mjs",
      "default": "./fesm2022/ngneat-spectator-jest.mjs"
    }
  },

and the new package.json has:

  "exports": {
    "./package.json": {
      "default": "./package.json"
    },
    ".": {
      "types": "./index.d.ts",
      "default": "./fesm2022/ngneat-spectator.mjs"
    },
    "./internals": {
      "types": "./internals/index.d.ts",
      "default": "./fesm2022/ngneat-spectator-internals.mjs"
    },
    "./jest": {
      "types": "./jest/index.d.ts",
      "default": "./fesm2022/ngneat-spectator-jest.mjs"
    }
  },

This is only a breaking change if a project has a hard-coded link to something in the /esm2022 folder, and doesn't use the standard ESM/package.json resolution mechanism to find the right reference.

No changes are required for normal projects, but this could require a change if someone has a build hack.

@johncrim johncrim changed the title Chore/ng19 update wout yarn Update to ng19 and set standalone property for all components Nov 26, 2024
CHANGELOG.md Outdated

* 🎸 Upgrade to Angular 19 ([7f3eb6c](https://github.com/ngneat/spectator/commit/7f3eb6c541fa9843a8e2f10ab8c77bca33a678c0))
Copy link
Member

Choose a reason for hiding this comment

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

please revert this file

Copy link
Contributor Author

@johncrim johncrim Nov 27, 2024

Choose a reason for hiding this comment

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

OK - I thought that I was just supposed to revert the version header line, not all my Changelog edits.

As I recall, for my last contribution I added some Changelog documentation, so that's what I was doing here. I'll revert all my changes to the file then.

@johncrim
Copy link
Contributor Author

@NetanelBasal - I believe this PR is ready to be merged when you have time.

@wilson-webdev
Copy link

@NetanelBasal - I believe this PR is ready to be merged when you have time.

I believe the changelog needs to be reverted first @johncrim

@johncrim
Copy link
Contributor Author

Thanks @wilson-webdev - all changes to the Changelog file have been removed.

@NetanelBasal NetanelBasal merged commit 7fe5d9b into ngneat:master Nov 27, 2024
3 checks passed
@wilson-webdev
Copy link

@NetanelBasal when will this be published on NPM?

@NetanelBasal
Copy link
Member

Published

@spiralx
Copy link

spiralx commented Nov 28, 2024

Published

Seems you might have published (the previously unpublished) version 19.1.0 to NPM and not 19.1.1:

image

@NetanelBasal
Copy link
Member

hmm, never mind, the code of both is there

@mattlewis92
Copy link

hmm, never mind, the code of both is there

@NetanelBasal Sorry to bother, but I don't think the latest is published to npm, for example the || {} was removed in the angular 19 patch, and yet is still present on the latest published version on npm:
https://unpkg.com/browse/@ngneat/[email protected]/esm2022/jest/lib/mock.mjs

@NetanelBasal
Copy link
Member

v19.1.2 should be good now

@mattlewis92
Copy link

v19.1.2 should be good now

Thank you so much, the new release works great with v19! 🙌

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