Skip to content

Production build errors with type-only imports inside curlys and emitDecoratorMetadata on #23667

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 15 tasks
jdforsythe opened this issue Aug 1, 2022 · 3 comments · Fixed by #23671
Closed
1 of 15 tasks
Assignees
Labels
area: @ngtools/webpack freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix

Comments

@jdforsythe
Copy link

jdforsythe commented Aug 1, 2022

🐞 Bug report

Command (mark with an x)

  • new
  • build
  • serve
  • test
  • e2e
  • generate
  • add
  • update
  • lint
  • extract-i18n
  • run
  • config
  • help
  • version
  • doc

Is this a regression?

Yes, the previous version in which this bug was not present was: ~~12.x.x~~ (we were using tslint before, so I'm not really sure)

Description

A clear and concise description of the problem...

🔬 Minimal Reproduction

  1. Run ng new test-app
  2. Apply the below patch, which just imports OnChanges and SimpleChanges as type-only imports and adds emitDecoratorMetadata: true to tsconfig.json
  3. Run npx ng build (succeeds on development build)
  4. Run npx ng build --configuration production:
./src/app/app.component.ts - Error: Module build failed (from ./node_modules/@angular-devkit/build-angular/src/babel/webpack-loader.js):
SyntaxError: /Users/********/dev/temp/test-type-modifier/src/app/app.component.ts: Unexpected token, expected "," (1:14)

> 1 | import { type SimpleChanges, } from '@angular/core';
    |               ^
  2 | import * as i0 from "@angular/core";
  3 | import * as i1 from "@angular/common";
  4 | import * as i2 from "@angular/router";

Patch for minimum reproduction:

diff --git a/src/app/app.component.ts b/src/app/app.component.ts
index 0e696fa..36f0caf 100644
--- a/src/app/app.component.ts
+++ b/src/app/app.component.ts
@@ -1,10 +1,18 @@
-import { Component } from '@angular/core';
+import {
+  Component,
+  type OnChanges,
+  type SimpleChanges,
+} from '@angular/core';
 
 @Component({
   selector: 'app-root',
   templateUrl: './app.component.html',
   styleUrls: ['./app.component.scss']
 })
-export class AppComponent {
+export class AppComponent implements OnChanges {
   title = 'test-type-modifier';
+
+  ngOnChanges(changes: SimpleChanges): void {
+    console.log(changes);
+  }
 }
diff --git a/tsconfig.json b/tsconfig.json
index ff06eae..b5be6c7 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -14,6 +14,7 @@
     "declaration": false,
     "downlevelIteration": true,
     "experimentalDecorators": true,
+    "emitDecoratorMetadata": true,
     "moduleResolution": "node",
     "importHelpers": true,
     "target": "es2020",

🔥 Exception or Error




./src/app/app.component.ts - Error: Module build failed (from ./node_modules/@angular-devkit/build-angular/src/babel/webpack-loader.js):
SyntaxError: /Users/******/dev/temp/test-type-modifier/src/app/app.component.ts: Unexpected token, expected "," (1:14)

> 1 | import { type SimpleChanges, } from '@angular/core';
    |               ^
  2 | import * as i0 from "@angular/core";
  3 | import * as i1 from "@angular/common";
  4 | import * as i2 from "@angular/router";

🌍 Your Environment




     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 14.1.0
Node: 16.15.0
Package Manager: npm 8.5.5 
OS: darwin arm64

Angular: 14.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1401.0
@angular-devkit/build-angular   14.1.0
@angular-devkit/core            14.1.0
@angular-devkit/schematics      14.1.0
@schematics/angular             14.1.0
rxjs                            7.5.6
typescript                      4.7.4

Anything else relevant?

The problem only exists with:

  1. A type-only import which uses the type keyword inside the curly braces
  2. emitDecoratorMetadata is true in tsconfig.json
  3. Certain type imports
  4. Production build

Details:

This problem happens when you use type-only imports (using the type keyword) using the syntax introduced in TypeScript 4.5 where you use the type keyword inside the curly braces:

import { type SimpleChanges } from '@angular/core';

and does not happen when you use the type keyword outside the curly braces:

import type { SimpleChanges } from '@angular/core';

We enable emitDecoratorMetadata in tsconfig.json due to eslint rules which require type information (otherwise it thinks DI classes in the constructor should be type-only imports, for example, but they can't be)

This problem only happens with certain imports, and only with @angular imports. Known failing types:

  • import { type SimpleChanges } from '@angular/core'
  • import { type Route } from '@angular/router'

Known types that don't cause the error:

  • import { type Routes, type CanLoad } from '@angular/router'
  • import { type OnInit, type OnChanges } from '@angular/core';
  • import { type HttpHeaders, type HttpParams } from '@angular/common/http'
  • any other npm module, e.g. import { type Observable } from 'rxjs' - this works fine
  • any local module, e.g. import { type MyInterface } from './interface' - this works fine

This problem only happens on production builds. Regular development builds and ng serve work without issue.

@jdforsythe
Copy link
Author

jdforsythe commented Aug 1, 2022

For anyone else who experiences this, there is a workaround.

Leave emitDecoratorMetadata set to true in tsconfig.json.
The CLI will use the regular tsconfig.app.json (with this setting set to true) for development and linting.

Create a new tsconfig.app.prod.json file:

{
  "extends": "./tsconfig.app.json",
  "compilerOptions": {
    "emitDecoratorMetadata": false
  }
}

And in angular.json under the build configuration for production, tell it to use this new tsconfig file:

{
  "projects": {
    "app": {
      "architect": {
        "build": {
          "configurations": {
            "production": {
              "tsConfig": "tsconfig.app.prod.json"
            }
          }
        }
      }
    }
  }
}

Alternatively, you can leave emitDecoratorMetadata set to false in tsconfig.app.json and turn it on only for linting in eslintrc.json:

{
  "root": true,
  "extends": [],
  "parserOptions": {
    "emitDecoratorMetadata": true
  }
}

@alan-agius4 alan-agius4 self-assigned this Aug 2, 2022
@alan-agius4 alan-agius4 added type: bug/fix freq1: low Only reported by a handful of users who observe it rarely area: @ngtools/webpack severity3: broken labels Aug 2, 2022
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue Aug 2, 2022
…DecoratorMetadata`

With this change we fix an issue where type only named imports were being emitted. As a result webpack failed to resolve such symbols as they don't exist in JavaScript.

Closes angular#23667
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue Aug 2, 2022
…DecoratorMetadata`

With this change we fix an issue where type only named imports were being emitted. As a result webpack failed to resolve such symbols as they don't exist in JavaScript.

Closes angular#23667
@clydin
Copy link
Member

clydin commented Aug 4, 2022

We are investigating a fix for the issue. However, emitDecoratorMetadata should generally be avoided when possible due to it having known design limitations that can lead to temporal dead zone errors during application runtime. These limitations are not Angular specific.
The last workaround mentioned above of adding the option into the eslint configuration is most likely the ideal option as it avoids the use of the option for built code.

dgp1130 pushed a commit that referenced this issue Aug 5, 2022
…DecoratorMetadata`

With this change we fix an issue where type only named imports were being emitted. As a result webpack failed to resolve such symbols as they don't exist in JavaScript.

Closes #23667
dgp1130 pushed a commit that referenced this issue Aug 5, 2022
…DecoratorMetadata`

With this change we fix an issue where type only named imports were being emitted. As a result webpack failed to resolve such symbols as they don't exist in JavaScript.

Closes #23667

(cherry picked from commit cf9afee)
@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 Sep 5, 2022
ikjelle pushed a commit to ikjelle/angular-cli that referenced this issue Mar 26, 2024
…DecoratorMetadata`

With this change we fix an issue where type only named imports were being emitted. As a result webpack failed to resolve such symbols as they don't exist in JavaScript.

Closes angular#23667
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 severity3: broken type: bug/fix
Projects
None yet
3 participants