sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Enable partial Ivy format for Angular

Open BojanKogoj opened this issue 3 years ago • 4 comments

Problem Statement

Angular 13+ will remind you of any library, that isn't compiled in partial Ivy and requires using ngcc.

Processing legacy "View Engine" libraries:
- @sentry/angular [es2015/esm2015] (git://github.com/getsentry/sentry-javascript.git)
Encourage the library authors to publish an Ivy distribution.

Any chance to go Ivy way? I know some users might not be using Ivy yet, but I'd like to get rid of ngcc as soon as possible.

Solution Brainstorm

In tsconfig remove "enableIvy": false.

BojanKogoj avatar Jun 16 '22 13:06 BojanKogoj

Hey, thanks for writing in. We've chatted about this in https://github.com/getsentry/sentry-javascript/pull/5253#issuecomment-1154834677, and will be addressing this in our next major version, v8. See https://github.com/getsentry/sentry-javascript/issues/5194#issuecomment-1157389443 also.

Backlogging this for now. If anyone else is interested in this (or is interested in having this happen before the next major), please leave a reaction to this issue or a comment in the issue itself.

AbhiPrasad avatar Jun 16 '22 14:06 AbhiPrasad

Hi! Is there any roadmap or date, when v8 (with Ivy support) will be released?

henningtillmann avatar Aug 08 '22 11:08 henningtillmann

No timelines for the next major at the moment, cc @vladanpaunovic

AbhiPrasad avatar Aug 08 '22 12:08 AbhiPrasad

@henningtillmann, thanks for your interest!

Sadly I don't have dates to share with you yet.

What I can tell you is that we are currently on v7.9.0 and I don't think it will take us long enough before we jump to version 8.

Once we know more, we will update the milestone.

vladanpaunovic avatar Aug 08 '22 12:08 vladanpaunovic

The version before v7.0.0 was like v6.19.0...

Out of curiosity, how do you decide whether to increment the minor version, versus the patch version? I'm looking at the release history and at a glance it seems inconsistent to me.

jpike88 avatar Nov 04 '22 09:11 jpike88

@jpike88 we follow semantic versioning, so if the release is fixes only we make it a patch, and if it includes new features we make it a minor. There are exceptions to this rule, but 95% of time we follow this.

AbhiPrasad avatar Nov 08 '22 17:11 AbhiPrasad

@vladanpaunovic - version 8.0: No due date, 7% complete

Seems like it will not happen in the nearest 6 months, right?

Lonli-Lokli avatar Nov 14 '22 11:11 Lonli-Lokli

Angular 15 is already available

Lonli-Lokli avatar Nov 30 '22 14:11 Lonli-Lokli

Hi everyone, just wanted to give you an update on this issue:

As I already described in https://github.com/getsentry/sentry-javascript/pull/5253#issuecomment-1154834677, we can only make the switch to Ivy in our next major, v8. We're currently trying to find out, how many of our users are still on Angular 10 and 11 to determine, if we can simple bump this package to support Angular 12+, which makes it possible for us to enable Ivy format. In case there's still a considerable amount of NG10 and 11 users, we'll still bump the @senrty/angular package to 12+ and provide a separate "legacy" package for them.

I'm fully aware that many of you want to see this happening sooner (I cannot give an ETA on v8) but for now I'm afraid the warning message will persist. However, we're interested in feedback here:

  • What is the impact of this warning message for you? Do you just want to get rid of it (fully understandable) or is there more serious impact in terms of e.g. performance?
  • If you have experience with Angular library development, is there a way we're not aware of, how we could introduce this change in a non-breaking way? IOW, can we enable Ivy support without impacting Angular 10 and 11 users at all in this very package?
  • If you are still using Angular 11 or older, is it likely that you'll upgrade to a newer version soon? If not, do you expect Sentry to continue support, although LTS support for your Angular version was dropped long ago?

Thanks for your patience and looking forward to reading your feedback.

Lms24 avatar Dec 09 '22 12:12 Lms24

Angular 15 is already available

Yup, we're aware and our SDK already supports it since version 7.20.0

Lms24 avatar Dec 09 '22 12:12 Lms24

  • What is the impact of this warning message for you? Do you just want to get rid of it (fully understandable) or is there more serious impact in terms of e.g. performance?

Just want to get rid of the warning message, but beyond that it doesn't have any performance impact I'm aware of.

  • If you have experience with Angular library development, is there a way we're not aware of, how we could introduce this change in a non-breaking way? IOW, can we enable Ivy support without impacting Angular 10 and 11 users at all in this very package?

From my understanding, this is only a change that application developers make regarding the AOT compiler and linker, but the ivy engine is the ivy engine. So it should be compatible as far back as v8 (which needed ivy to be opt-in enabled)

jpike88 avatar Dec 09 '22 12:12 jpike88

  • What is the impact of this warning message for you? Do you just want to get rid of it (fully understandable) or is there more serious impact in terms of e.g. performance?

IIUC there is a performance impact in terms of the build performance, since view engine libraries require an additional step of analysis/conversion at build time. The results of this step are cached locally so the impact should be negligible in the developers' environment, but it likely to be present in CI/CD pipelines (unless additional care is taken to manage the cache).

ranma42 avatar Dec 09 '22 13:12 ranma42

  • What is the impact of this warning message for you? Do you just want to get rid of it (fully understandable) or is there more serious impact in terms of e.g. performance?

My app is a bigger web component, i.e. bundle size is very important. Every byte saved helps. Therefore, Ivy is a big help and mandatory.

henningtillmann avatar Dec 09 '22 14:12 henningtillmann

  • What is the impact of this warning message for you? Do you just want to get rid of it (fully understandable) or is there more serious impact in terms of e.g. performance?

The issue is, that we still need ngcc to make it compatibly with Ivy. In our company's app, @sentry/angular is the last dependency that is still on View Engine, so we cannot get rid of ngcc for now. And that has some downsides for us, because for example we want to switch from npm to pnpm for performance reasons. But the need for ngcc is hindering us from doing this, because it introduces issues for running parallel builds when your workspace is build with pnpm.

So it's not just about the warning, but it has some deeper impacts on other things. I'm really looking forward to when @sentry/angular is finally on Ivy :)

decline avatar Dec 16 '22 09:12 decline

Any updates ? Do you have an ETA ?

I know you want to wait for v8 to drop support for Angular v11 and lower but it's only 25% completed. Meanwhile, Angular v15.2.0 will be released soon.

jdussouillez avatar Jan 27 '23 08:01 jdussouillez

We are in the same boat as @decline. Also want to move away from yarn v1 but it's really difficult having to keep running ngcc to only compile @sentry/angular ):

alfaproject avatar Feb 04 '23 15:02 alfaproject

Another reason we would like this sooner rather than later is that the esbuild builderfor Angular is now more capable and could be probably enough for our project, and way faster. But it only support Ivy

mredaelli avatar Feb 15 '23 10:02 mredaelli

There is a PR for probably NG16, which will remove ngcc and make Ivy the only option https://github.com/angular/angular-cli/pull/24720

Lonli-Lokli avatar Feb 15 '23 20:02 Lonli-Lokli

Hey everyone, considering that v8 is gonna take a little longer to land, we decided last week to create a new Ivy-compatible Angular SDK package. Gonna start working on this soon. Stay tuned.

Lms24 avatar Feb 16 '23 06:02 Lms24

Hey everyone, considering that v8 is gonna take a little longer to land, we decided last week to create a new Ivy-compatible Angular SDK package. Gonna start working on this soon. Stay tuned.

Yay!

yharaskrik avatar Feb 17 '23 15:02 yharaskrik

Hey folks, #7264 is currently undergoing reviews and will soon be released as an experimental package. If you have thoughts on the package (e.g. build configuration), feel free to drop a note in the PR.

We're tracking everything that's related to creating and publishing this new package in #7265.

Lms24 avatar Feb 24 '23 08:02 Lms24

We just released version 7.39.0 with @sentry/angular-ivy. I would really appreciate if folks following this thread would give it a try and report back if anything isn't working as expected. Please open a new issue, if you encounter a bug.

Migration from @sentry/angular to @sentry/angular-ivy:

  1. Remove @sentry/angular from your dependencies
  2. Add @sentry/angular-ivy as a dependency (available in version ^7.39.0)
  3. Replace all imports from @sentry/angular with @sentry/angular-ivy

Please make sure that all you @sentry/* packages are aligned to the same version (^7.39.0)

We're currently working on official docs but generally everything should work just like it did in @sentry/angular. For anyone interested, this PR tracks the docs changes.

Lms24 avatar Feb 27 '23 15:02 Lms24

Hi @Lms24 I;ve updated to the latest version and switched to the ivy option, my build is failing

import { createErrorHandler, init as initSentry, routingInstrumentation, TraceService } from '@sentry/angular-ivy';
import { Integrations } from '@sentry/tracing';
...
if (environment.production) {
  enableProdMode();

  initSentry({
    dsn: 'UURL',
    integrations: [
      new Integrations.BrowserTracing({
        tracingOrigins: ['URL'],
        routingInstrumentation: routingInstrumentation
      })
    ],
    release: environment.build.version,
    tracesSampleRate: 0.25
  });
}

under strict: true

 Type 'import("D:/gitlab/cv-app/frontend/cv-ui/node_modules/@sentry/types/types/context").Contexts' is not assignable to type 'import("D:/gitlab/cv-app/frontend/cv-ui/node_modules/@sentry/hub/node_modules/@sentry/types/types/context").Contexts'.
                      'string' index signatures are incompatible.
                        Type 'Context | undefined' is not assignable to type 'Context'.
                          Type 'undefined' is not assignable to type 'Context'.

Error: node_modules/@sentry/hub/node_modules/@sentry/types/types/globals.d.ts:2:11 - error TS2451: Cannot redeclare block-scoped variable '__DEBUG_BUILD__'.

2     const __DEBUG_BUILD__: boolean;
            ~~~~~~~~~~~~~~~

  node_modules/@sentry/types/types/globals.d.ts:2:11
    2     const __DEBUG_BUILD__: boolean;
                ~~~~~~~~~~~~~~~
    '__DEBUG_BUILD__' was also declared here.


Error: node_modules/@sentry/types/types/globals.d.ts:2:11 - error TS2451: Cannot redeclare block-scoped variable '__DEBUG_BUILD__'.

2     const __DEBUG_BUILD__: boolean;
            ~~~~~~~~~~~~~~~

  node_modules/@sentry/hub/node_modules/@sentry/types/types/globals.d.ts:2:11
    2     const __DEBUG_BUILD__: boolean;
                ~~~~~~~~~~~~~~~
    '__DEBUG_BUILD__' was also declared here.

Lonli-Lokli avatar Feb 27 '23 15:02 Lonli-Lokli

As an additional note, package.json's have different locked versions, is it expected? angular-ivy

"dependencies": {
    "@sentry/browser": "7.39.0",
    "@sentry/types": "7.39.0",
    "@sentry/utils": "7.39.0",
    "tslib": "^2.3.0"
  },

browser

"dependencies": {
    "@sentry/core": "7.39.0",
    "@sentry/replay": "7.39.0",
    "@sentry/types": "7.39.0",
    "@sentry/utils": "7.39.0",
    "tslib": "^1.9.3"
  },

core

 "dependencies": {
    "@sentry/types": "7.39.0",
    "@sentry/utils": "7.39.0",
    "tslib": "^1.9.3"
  },

hub

"dependencies": {
    "@sentry/types": "7.3.0",
    "@sentry/utils": "7.3.0",
    "tslib": "^1.9.3"
  },

replay

"dependencies": {
    "@sentry/core": "7.39.0",
    "@sentry/types": "7.39.0",
    "@sentry/utils": "7.39.0"
  },

tracing

  "dependencies": {
    "@sentry/hub": "7.3.0",
    "@sentry/types": "7.3.0",
    "@sentry/utils": "7.3.0",
    "tslib": "^1.9.3"
  },

utils

 "dependencies": {
    "@sentry/types": "7.39.0",
    "tslib": "^1.9.3"
  },

Lonli-Lokli avatar Feb 27 '23 15:02 Lonli-Lokli

@Lonli-Lokli thanks for trying it out.

As an additional note, package.json's have different locked versions, is it expected

No, this is definitely not expected. All packages must be aligned on the same version. Seems like the @sentry/tracing package is still on 7.3.0 which causes these errors. Also, you should be good to delete the @sentry/hub package as we don't need it anymore (all its contents were moved to @sentry/core).

Lms24 avatar Feb 27 '23 16:02 Lms24

@Lms24 Hold on, maybe it's my mistake :-) I see that not all refs has been updated

EDIT: Yes, it was my mistake, sorry about that. All refs except for tslib in angular-ivy are the same now.

Lonli-Lokli avatar Feb 27 '23 16:02 Lonli-Lokli