Skip to content

ngc emits JS for Typescript vendor files #20115

Closed
Closed
ngc emits JS for Typescript vendor files#20115
Labels
area: compilerIssues related to `ngc`, Angular's template compilerfreq3: highneeds reproductionThis issue needs a reproduction in order for the team to investigate furtherstate: confirmedtype: bug/fix
Milestone
Backlog

Description

@filipesilva
filipesilva
opened on Nov 2, 2017

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

ngc will emit JavaScript files for vendor TypeScript files imported by the user app, even if these files are not part of the files included by tsconfig.

Expected behavior

Files that are not included in tsconfig should not be compiled.

Minimal reproduction of the problem with instructions

git clone https://github.com/filipesilva/bad-vendor-ngc
cd bad-vendor-ngc
npm i
npm run ngc

After the ngc compilation is finished, you can see non-factory JS files emitted for the angular2-click-outside library:

$ ls -lha out-tsc/app/node_modules/angular2-click-outside/
total 25K
drwxr-xr-x 1 kamik 197609    0 Nov  2 17:20 .
drwxr-xr-x 1 kamik 197609    0 Nov  2 17:20 ..
-rw-r--r-- 1 kamik 197609 1.3K Nov  2 17:20 clickOutside.directive.js
-rw-r--r-- 1 kamik 197609  761 Nov  2 17:20 clickOutside.directive.js.map
-rw-r--r-- 1 kamik 197609  782 Nov  2 17:20 clickOutside.directive.metadata.json
-rw-r--r-- 1 kamik 197609  197 Nov  2 17:20 clickOutside.directive.ngfactory.js.map
-rw-r--r-- 1 kamik 197609 1.5K Nov  2 17:20 clickOutside.directive.ngsummary.json

This library was published with TS files, and without metadata:

$ ls -lha node_modules/angular2-click-outside/
total 295K
drwxr-xr-x 1 kamik 197609    0 Nov  2 17:16 .
drwxr-xr-x 1 kamik 197609    0 Nov  2 17:16 ..
drwxr-xr-x 1 kamik 197609    0 Nov  2 17:16 .idea
-rw-r--r-- 1 kamik 197609   21 May 15  2016 .npmignore
-rw-r--r-- 1 kamik 197609    3 May 15  2016 .nvmrc
-rw-r--r-- 1 kamik 197609  282 Aug 19  2016 clickOutside.directive.d.ts
-rw-r--r-- 1 kamik 197609 2.8K Aug 19  2016 clickOutside.directive.js
-rw-r--r-- 1 kamik 197609 1.5K Aug 19  2016 clickOutside.directive.js.map
-rw-r--r-- 1 kamik 197609  682 Aug 19  2016 clickOutside.directive.ts
-rw-r--r-- 1 kamik 197609  601 May 15  2016 gulpfile.js
-rw-r--r-- 1 kamik 197609   66 Aug 19  2016 index.d.ts
-rw-r--r-- 1 kamik 197609  476 Aug 19  2016 index.js
-rw-r--r-- 1 kamik 197609  113 Aug 19  2016 index.js.map
-rw-r--r-- 1 kamik 197609   64 May 15  2016 index.ts
-rw-r--r-- 1 kamik 197609 1.5K May 15  2016 LICENSE
-rw-r--r-- 1 kamik 197609 1.7K Nov  2 17:16 package.json
-rw-r--r-- 1 kamik 197609  368 May 16  2016 README.md
-rw-r--r-- 1 kamik 197609  368 May 15  2016 tsconfig.json
-rw-r--r-- 1 kamik 197609   96 May 15  2016 typings.json

What is the motivation / use case for changing the behavior?

This behaviour is problematic because it hides improperly packaged libraries, and may fail compilation due to the app TS version being different from the TS version needed by the library.

Environment


Angular version: 5.0.0


Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: XX   6.11.1
- Platform:   Windows

Others:

Related to angular/angular-cli#8263, angular/angular-cli#8216.

/cc @chuckjaz

Activity

  1. chuckjaz
    added
    area: coreIssues related to the framework runtime
    type: bug/fix
    on Nov 2, 2017

  2. tbosch commented on Nov 2, 2017

    @tbosch
    tbosch
    on Nov 2, 2017
    Contributor

    Analysis:

    • ngc includes .ts files into the compilation, as ngc incorrectly tells TS that this file is not in node_modules (via ts.ResolvedModule.isExternalModule = false).

    Proposed fix:

    • change CompilerHost.resolveModuleName to only set isExternalModule to false if the file is a this.isSourceFile and it is a .d.ts file. We can't remove this logic altogether as otherwise our generated files for files in node_modules would not be emitted by TypeScript as TypeScript thinks they are not sources, as the file the original file in node_modules is not.
    • this is the same fix as for AOT compilation with allowJs: true fails in 5.0.0-rc.2 #19757
  3. mhevery
    added
    freq3: high
    severity3: broken
    on Nov 30, 2017

  4. delagen commented on Jan 11, 2018

    @delagen
    delagen
    on Jan 11, 2018
    Contributor

    Seems this issue broke my build? Version 5.1.1 works fine. But latest does not generate JS factories, but emit metadata and definitions for files in node_modules

  5. AllNamesRTaken commented on Jan 15, 2018

    @AllNamesRTaken
    AllNamesRTaken
    on Jan 15, 2018

    I am running 5.2.0 and using ngx-boostrap and this seem related to the fact that there is no X.component.js and only X.component.ngfactory.js in the generated node_modules folder. But the factories references have been rewritten to local paths making it impossible to bundle (using rollup).

    The issue is well described here.

    Either we have to keep the js files or not rewrite imports for those js files.

  6. mlc-mlapis commented on Jan 15, 2018

    @mlc-mlapis
    mlc-mlapis
    on Jan 15, 2018
    Contributor

    @AllNamesRTaken ... what are you using for building the lib?

  7. AllNamesRTaken commented on Jan 15, 2018

    @AllNamesRTaken
    AllNamesRTaken
    on Jan 15, 2018

    @mlc-mlapis I am using ngc.

    node_modules\.bin\ngc -p src\tsconfig.json

    EDIT

    It does work if I add this to tsconfig.json which indicates that it is related.

    "include": [
	"./main.ts",
	"../node_modules/ngx-bootstrap/**/*.component.js",
	"../node_modules/ngx-bootstrap/**/*.class.js"
],

    This forces me to for each such library add a number of specific includes which is less than optimal.

  8. mlc-mlapis commented on Jan 15, 2018

    @mlc-mlapis
    mlc-mlapis
    on Jan 15, 2018
    Contributor

    @AllNamesRTaken ... hmm, we are using ng-packagr for building our packages due to the specs of Angular Package Format 5.0. Later when compiling an app ... those packages are already installed in node_packages as all others packages. And ... there are *.js versions of that packages already. NGC is able then to compile those packages to AOT mode ... so *.ngfactory.js are created. And finally ... we create bundles ... *.js and *.ngfactory.js are included to (+ we eliminate decorators because we don't need them in AOT mode).

    With the above steps we don't need to add anything to includesection of tsconfig.json file.

  9. AllNamesRTaken commented on Jan 15, 2018

    @AllNamesRTaken
    AllNamesRTaken
    on Jan 15, 2018

    @mlc-mlapis But that does sound like you have a workaround. My point is that a clean ngc should not generate broken files.

    Either it should not mangle the path, or it should add those js files that are imported using es2015 module format. That you can get it to work I am sure, but it feels like the law of least surprise is not in action here :).

  10. ngbot
    added this to the Backlog milestone on Jan 23, 2018
  11. ngbot
    modified the milestones: Backlog, needsTriage on Feb 26, 2018
  12. pkozlowski-opensource
    added
    area: compilerIssues related to `ngc`, Angular's template compiler
    and removed
    area: coreIssues related to the framework runtime
    on Mar 12, 2020
  13. alxhub
    added
    engine: ve
    needs reproductionThis issue needs a reproduction in order for the team to investigate further
    state: confirmed
    triage #1
    on May 28, 2020

  14. alxhub commented on May 28, 2020

    @alxhub
    alxhub
    on May 28, 2020
    Member

    This has since been fixed - tested with Angular v10 RC.0.

  15. alxhub
    closed this as completedon May 28, 2020

  16. angular-automatic-lock-bot commented on Jun 29, 2020

    @angular-automatic-lock-bot
    angular-automatic-lock-botBot
    on Jun 29, 2020

    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.

  17. angular-automatic-lock-bot
    locked and limited conversation to collaborators on Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: compilerIssues related to `ngc`, Angular's template compilerfreq3: highneeds reproductionThis issue needs a reproduction in order for the team to investigate furtherstate: confirmedtype: bug/fix

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions