Skip to content

property shorthand usage for animations cause errors in production build #17347

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

Closed
1 of 2 tasks
crysislinux opened this issue Mar 31, 2020 · 2 comments · Fixed by #18158
Closed
1 of 2 tasks

property shorthand usage for animations cause errors in production build #17347

crysislinux opened this issue Mar 31, 2020 · 2 comments · Fixed by #18158
Labels
area: @ngtools/webpack freq1: low Only reported by a handful of users who observe it rarely severity5: regression type: bug/fix
Milestone

Comments

@crysislinux
Copy link

🐞 Bug report

Command (mark with an x)

  • new
  • build

Is this a regression?

Yes

Our code ran without problem with --prod build flag in angular 7.

But now it fails with the same flag in angular 9 with ivy enabled.

Description

We have some shared animations definitions like this:

animations.ts

import { trigger, transition, style, group, animate } from '@angular/animations';

export const animations = [
  trigger('expandIn', [
    transition(':enter', [
      style({
        overflow: 'hidden',
        height: 0,
        paddingTop: 0,
        paddingBottom: 0,
        marginTop: 0,
        marginBottom: 0,
        opacity: 0
      }),
      group([
        animate('500ms cubic-bezier(0.23, 1, 0.32, 1)', style({
          height: '*', paddingTop: '*', paddingBottom: '*', marginTop: '*', marginBottom: '*', transform: 'translateZ(0)'
        })),
        animate('250ms cubic-bezier(0.23, 1, 0.32, 1)', style({ opacity: 1 }))
      ])
    ]),
  ]),
];

the animations are defined in a separate file animations.ts. And then it's referenced like this:

import { Component, OnInit } from '@angular/core';
import { animations } from './animations';

@Component({
  selector: 'app-notlazycomp',
  templateUrl: './notlazycomp.component.html',
  styleUrls: ['./notlazycomp.component.scss'],
  animations,  // notice this property shorthand syntax
})
export class NotLazycompComponent implements OnInit {

  constructor() { }

  ngOnInit(): void {
  }

}

Those codes look good to me and they works in Angular 7. Now with Angular 9, the production build will throw runtime error:

image

I have found 3 ways to workaround it:

  1. disable ivy with "enableIvy": false,
  2. set "optimization": false, in production configuration (aot and buildOptimizer can still be true)
  3. abandon property shorthand for animations property, just change to animations: animations also fixes the problem.

We went with option 3 but it looks confusing why valid typescript code doesn't work and the linter even tell me to use property shorthand. It's fine to be a compilation error but unacceptable to be a runtime error only.

🔬 Minimal Reproduction

  1. clone repo: https://github.com/crysislinux/angular9-prod-build-runtime-error
  2. npm run build to create production build
  3. http-server dist/angular9-ext -p 4200
  4. visit http://localhost:4200/

You should see some error messages in the console.

  1. switch optimization from true to false in angular.json line 43
  2. repeat step 2 to 4

Now it loads without problem.

  1. switch optimization from false to true
  2. repeat step 2 to 4

Now you should see the problem again

  1. edit src/app/notlazy/notlazycomp/notlazycomp.component.ts, change line 8 from animations, to animations: animations,
  2. repeat step 2 to 4

Now it loads without problem.

Turn off ivy also fix the problem but I have a lazy load route in the repo so it may not work. By the way, this problem also breaks the lazy loading if there is the same animations property shorthand usage in the lazy loaded module.

🔥 Exception or Error

image

🌍 Your Environment


Angular CLI: 9.1.0
Node: 10.16.0
OS: darwin x64

Angular: 9.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.0
@angular-devkit/build-angular     0.901.0
@angular-devkit/build-optimizer   0.901.0
@angular-devkit/build-webpack     0.901.0
@angular-devkit/core              9.1.0
@angular-devkit/schematics        9.1.0
@ngtools/webpack                  9.1.0
@schematics/angular               9.1.0
@schematics/update                0.901.0
rxjs                              6.5.4
typescript                        3.7.5
webpack                           4.42.0

Anything else relevant?

N/A

@alan-agius4 alan-agius4 added area: @angular-devkit/build-angular freq1: low Only reported by a handful of users who observe it rarely severity5: regression labels Mar 31, 2020
@ngbot ngbot bot modified the milestone: needsTriage Mar 31, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Mar 31, 2020
@getabetterpic
Copy link

We've run into this exact same issue. Right now I'm also going through and replacing these shorthand instances with animations: animations but it worries me that none of our CI builds caught this. It would be possible for one of these to slip in and we wouldn't know until something broke in prod.

alan-agius4 added a commit that referenced this issue Jul 6, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…handPropertyAssignment

NGTSC, will transform `ShorthandPropertyAssignment` to `PropertyAssignment`, with this change we handle such cases and retain the import which previously was dropped.

Closes #18149 and closes #17347
alan-agius4 added a commit that referenced this issue Jul 6, 2020
…handPropertyAssignment

NGTSC, will transform `ShorthandPropertyAssignment` to `PropertyAssignment`, with this change we handle such cases and retain the import which previously was dropped.

Closes #18149 and closes #17347

(cherry picked from commit 6ac000f)
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 6, 2020
ikjelle pushed a commit to ikjelle/angular-cli that referenced this issue Mar 26, 2024
…handPropertyAssignment

NGTSC, will transform `ShorthandPropertyAssignment` to `PropertyAssignment`, with this change we handle such cases and retain the import which previously was dropped.

Closes angular#18149 and closes angular#17347
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: @ngtools/webpack freq1: low Only reported by a handful of users who observe it rarely severity5: regression type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants