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

fix(@angular-devkit/build-angular): downlevel class properties when targeting Safari <=v15 #24357

Merged
merged 1 commit into from
Dec 2, 2022

Commits on Dec 1, 2022

  1. fix(@angular-devkit/build-angular): downlevel class properties when t…

    …argeting Safari <=v15
    
    The Angular compiler is dependent on static fields being attached to
    user-defined classes. e.g. `static ecmp = defineComponent`.
    
    These static fields sometimes rely on variables from outside of the
    class. e.g. the Angular compiler generates constants for content
    projection that are then accessed in the static field initializer.
    
    Surprisingly such access to these variables may break in Safari <=v15
    when a page is loaded without devtools open. The bug (already solved in
    v16 of Safari)- is very subtle, hard to re-reproduce but basically
    variable scope tracking is broken. This bug is triggered by additional
    parenthesis in the initializer expression. See:
    https://bugs.webkit.org/show_bug.cgi?id=236843.
    
    The TypeScript compiler may generate such additional parenthesis when
    it tries to adjust the `this` context when invoking methods, such as for
    defining animations in the `ecmp` definition.
    
    More details can be found here:
    angular#24355 (comment)
    
    To ensure Angular applications are not subject to this bug when
    targeting Safari <=v15. v15 Safari, both for iOS and Mac is still part of
    the default CLI browserslist with `last 2 Safari majors` (at time of
    writing).
    
    Note that it is important that the Babel plugin properly handles the
    downleveling of static block-defined members. TypeScript will transform
    static fields, like `static ecmp` into `static { this.ecmp = X }` when
    `useDefineForClassFields = false` (which is the case for CLI apps). The
    class properties plugin from Babel seems to handle this in an acceptable
    way. Unlike actual static fields, Babel will not use helpers like
    `defineProperty` for such extracted static blocks though. e.g.
    
    See repro: https://gist.github.com/devversion/dec0dea26e348c509921bf62079b60be
    
    ```js
    class Test {
      x = true;
    
      static b = true;
      static {
        this.a = true;
      }
    }
    
    // into
    
    class X {
      constructor() {
        _defineProperty(this, "x", true);
      }
    }
    _defineProperty(X, "b", true);
    X.a = true;
    ```
    
    note that in practice TypeScript with `useDefineForClassFields = false`
    will put non-static members into the constructor as normal assignments
    regardless- so there would be no change by the Babel plugin.
    
    Fixes angular#24355.
    devversion committed Dec 1, 2022
    Configuration menu
    Copy the full SHA
    86b19c5 View commit details
    Browse the repository at this point in the history