Skip to content

ivy-ngcc doesn't transform UMD bundles correctly #31394

New issue
New issue
Closed
Closed
ivy-ngcc doesn't transform UMD bundles correctly#31394
Assignees
JoostK
Labels
state: has PRtype: bug/fix
Milestone
needsTriage

Description

@gregmagolan
gregmagolan
opened on Jul 3, 2019

🐞 bug report

Affected Package

The issue is caused by ivy-ngcc tool

Is this a regression?

no

Description

ivy-ngcc transforms UMD bundles incorrectly. In particular, the symbol Injectable ends up as undefined in the transformed UMD bundle.

    AriaDescriber.decorators = [
        { type: core.Injectable, args: [{ providedIn: 'root' },] },
    ];
    /** @nocollapse */
    AriaDescriber.ctorParameters = function () { return [
        { type: undefined, decorators: [{ type: core.Inject, args: [common.DOCUMENT,] }] }
    ]; };
    /** @nocollapse */ AriaDescriber.ngInjectableDef = core.ɵɵdefineInjectable({ factory: function AriaDescriber_Factory() { return new AriaDescriber(core.ɵɵinject(common.DOCUMENT)); }, token: AriaDescriber, providedIn: "root" });
    return AriaDescriber;

in node_modules/@angular/cdk/bundles/cdk-a11y.umd.js.__ivy_ngcc_bak gets transformed to

/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(AriaDescriber, [{
        type: Injectable,
        args: [{ providedIn: 'root' }]
    }], function () { return [{ type: undefined, decorators: [{
                type: Inject,
                args: [common.DOCUMENT]
            }] }]; }, { _document: [], describe: [], removeDescription: [], ngOnDestroy: [], _createMessageElement: [], _deleteMessageElement: [], _createMessagesContainer: [], _deleteMessagesContainer: [], _removeCdkDescribedByReferenceIds: [], _addMessageReference: [], _removeMessageReference: [], _isElementDescribedByMessage: [], _canBeDescribed: [], _isElementNode: [] });
    return AriaDescriber;

in node_modules/@angular/cdk/bundles/cdk-a11y.umd.js where type: Injectable, is undefined when run in the browser:

Uncaught ReferenceError: Injectable is not defined

I would guess it should be type: core.Injectable since core is the @angular/core UMD dependency.

@angular/cdk/bundles/cdk-a11y.umd.js is just an example but same thing happens in other bundles transformed.

🔬 Reproduction

Clone the wassim/ivy branch of my fork of angular-bazel-example (https://github.com/gregmagolan/angular-bazel-example/tree/wassim/ivy) and run yarn install. ivy-ngcc is run in the postinstall step and you can inspect the resulting transofmed bundles under node_modules.

https://stackblitz.com/...

🔥 Exception or Error

🌍 Your Environment

Angular Version:

Anything else relevant?

Activity

  1. AndrewKushnir
    added
    comp: ivy
    comp: ngcc
    on Jul 3, 2019
  2. ngbot
    modified the milestone: needsTriage on Jul 3, 2019

  3. alexzuza commented on Jul 3, 2019

    @alexzuza
    alexzuza
    on Jul 3, 2019
    Contributor

    The same also happens with @angular/* umd bundles.

    I think the problem is located here

    angular/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts

    Lines 124 to 127 in 21835af

    function decoratorToMetadata(decorator: Decorator): ts.ObjectLiteralExpression {
    // Decorators have a type.
    const properties: ts.ObjectLiteralElementLike[] = [
    ts.createPropertyAssignment('type', ts.updateIdentifier(decorator.identifier)),

    If I check for property access then it works for me:

     function decoratorToMetadata(decorator) {
  const identifier = ts.isPropertyAccessExpression(decorator.identifier.parent) ? decorator.identifier.parent : decorator.identifier
  // Decorators have a type.
  var properties = [
    ts.createPropertyAssignment('type', ts.updateIdentifier(identifier)),
  ];

    @alxhub Can you please take a look at this?

  4. petebacondarwin commented on Jul 3, 2019

    @petebacondarwin
    petebacondarwin
    on Jul 3, 2019
    Contributor

    I'll take a look.

  5. petebacondarwin
    self-assigned this
    on Jul 3, 2019
  6. petebacondarwin
    added
    type: bug/fix
    on Jul 3, 2019
  7. petebacondarwin
    added a commit that references this issue on Jul 5, 2019

    fix(ivy): ngcc - render namespaced imported decorators correctly

    8ce247e

  8. petebacondarwin commented on Jul 5, 2019

    @petebacondarwin
    petebacondarwin
    on Jul 5, 2019
    Contributor

    Hopefully #31426 should fix this.

  9. petebacondarwin
    added
    state: has PR
    on Jul 8, 2019
  10. mhevery
    closed this as completedin dd664f6on Jul 18, 2019
  11. mhevery
    added a commit that references this issue on Jul 18, 2019

    fix(ivy): ngcc - render namespaced imported decorators correctly (#31426

    4e2db39

  12. gregmagolan commented on Jul 19, 2019

    @gregmagolan
    gregmagolan
    on Jul 19, 2019
    ContributorAuthor

    @petebacondarwin Thanks for the fix. I have good news and bad news. Will start with the good 🚀

    Good) Your fix (landed in master & 8.1.x branch) fixed the issue observed for all of the angular bundles. The ivy-ngcc generated file node_modules/@angular/cdk/bundles/cdk-a11y.umd.js file now has:

        /** @nocollapse */ AriaDescriber.ngInjectableDef = core.ɵɵdefineInjectable({ factory: function AriaDescriber_Factory() { return new AriaDescriber(core.ɵɵinject(common.DOCUMENT)); }, token: AriaDescriber, providedIn: "root" });
AriaDescriber.ngInjectableDef = ɵngcc0.ɵɵdefineInjectable({ token: AriaDescriber, factory: function AriaDescriber_Factory(t) { return new (t || AriaDescriber)(ɵngcc0.ɵɵinject(common.DOCUMENT)); }, providedIn: 'root' });
/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(AriaDescriber, [{
        type: core.Injectable,
        args: [{ providedIn: 'root' }]
    }], function () { return [{ type: undefined, decorators: [{
                type: core.Inject,
                args: [common.DOCUMENT]
            }] }]; }, { _document: [], describe: [], removeDescription: [], ngOnDestroy: [], _createMessageElement: [], _deleteMessageElement: [], _createMessagesContainer: [], _deleteMessagesContainer: [], _removeCdkDescribedByReferenceIds: [], _addMessageReference: [], _removeMessageReference: [], _isElementDescribedByMessage: [], _canBeDescribed: [], _isElementNode: [] });
    return AriaDescriber;

    which correctly has type: core.Injectable,

    Bad) The @ngrx umd bundles (from latest 8.1.0 release) still have the same error originally observed in the angular bundles

            ActionsSubject = tslib_1.__decorate([
            core.Injectable(),
            tslib_1.__metadata("design:paramtypes", [])
        ], ActionsSubject);
        return ActionsSubject;

    in node_modules/@ngrx/store/bundles/store.umd.js.__ivy_ngcc_bak is transformed to

    ActionsSubject.ngInjectableDef = ɵngcc0.ɵɵdefineInjectable({ token: ActionsSubject, factory: function ActionsSubject_Factory(t) { return new (t || ActionsSubject)(); }, providedIn: null });
/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(ActionsSubject, [{
        type: Injectable
    }], function () { return []; }, { next: [], complete: [], ngOnDestroy: [] });
        return ActionsSubject;

    in node_modules/@ngrx/store/bundles/store.umd.js which has type: Injectable again and it fails in the browser at runtime.

    There is a slight difference in that the ngrx bundle, which has core.Injectable(), while the issue observed with the cdk bundle node_modules/@angular/cdk/bundles/cdk-a11y.umd.js.__ivy_ngcc_bak had type: core.Injectable,.

  13. gregmagolan
    reopened this on Jul 19, 2019

  14. petebacondarwin commented on Jul 19, 2019

    @petebacondarwin
    petebacondarwin
    on Jul 19, 2019
    Contributor

    I think the difference is that one uses the static property style of decoration and the other uses the call helper style.

        AriaDescriber.decorators = [
        { type: core.Injectable, args: [{ providedIn: 'root' },] },
    ];

    vs

            ActionsSubject = tslib_1.__decorate([
            core.Injectable(),
            tslib_1.__metadata("design:paramtypes", [])
        ], ActionsSubject);

    I guess we only fixed it for the static property style.

  15. petebacondarwin
    removed
    state: has PR
    on Jul 19, 2019
  16. JoostK
    assigned
    JoostK
    and unassigned
    petebacondarwin
    on Jul 24, 2019
  17. JoostK
    added
    state: has PR
    on Jul 27, 2019

  18. JoostK commented on Jul 27, 2019

    @JoostK
    JoostK
    on Jul 27, 2019
    Member

    I can confirm this was only fixed for the static properties syntax, not the __decorate syntax. I included a fix in #31614 which contains some more fixes to deal with decorators correctly.

  19. JoostK
    added a commit that references this issue on Jul 27, 2019

    fix(ivy): ngcc - render decorators in UMD and CommonJS bundles correctly

    3ddd6bc
  20. AndrewKushnir
    closed this as completedin fc6f481on Jul 29, 2019

  21. gregmagolan commented on Aug 15, 2019

    @gregmagolan
    gregmagolan
    on Aug 15, 2019
    ContributorAuthor

    Confirmed that all the UMD bundles used by angular-bazel-example (including @ngrx which was using core.Injectable()) are correctly transformed now with ivy-ngcc from angular 9.0.0-next.2. Tested in angular/angular-bazel-example#505. Thanks @petebacondarwin @JoostK!

    @alexeagle @CaerusKaru ^^

  22. sabeersulaiman
    added 2 commits that reference this issue on Sep 6, 2019

    fix(ivy): ngcc - render namespaced imported decorators correctly (ang…

    433a281

    fix(ivy): ngcc - render decorators in UMD and CommonJS bundles correc…

    4e5df1f

  23. angular-automatic-lock-bot commented on Sep 15, 2019

    @angular-automatic-lock-bot
    angular-automatic-lock-botBot
    on Sep 15, 2019

    This issue has been automatically locked due to inactivity.
    Please file a new issue if you are encountering a similar or related problem.

    Read more about our automatic conversation locking policy.

    This action has been performed automatically by a bot.

  24. angular-automatic-lock-bot
    locked and limited conversation to collaborators on Sep 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

state: has PRtype: bug/fix

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions