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

SimpleNotificationsModule fails to load with angular-cli v1.5.0-rc8 under AOT #8263

Closed
asadsahi opened this issue Oct 31, 2017 · 15 comments · Fixed by flauc/angular2-notifications#261
Labels
P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix

Comments

@asadsahi
Copy link

Bug Report or Feature Request (mark with an x)

- [x ] bug report -> please search issues before submitting
- [ ] feature request

Versions.

Angular CLI: 1.5.0-rc.8
Node: 8.8.1
OS: win32 x64
Angular:
...

Repro steps.

with latest version of angular-cli(v1.5.0-rc.8) and angular 5.0.0-rc.9 with aot getting following error


ERROR in ./node_modules/angular2-notifications/src/simple-notifications/components/simple-notifications.component.ngfactory.ts
Module build failed: Error: C:\Users\Asad\dev\ngcli150\node_modules\angular2-notifications\src\simple-notifications\components\simple-notifications.component.ngfactory.ts is not part of the
compilation. Please make sure it is in your tsconfig via the 'files' or 'include' property.
    at AngularCompilerPlugin.getCompiledFile (C:\Users\Asad\dev\ngcli150\node_modules\@ngtools\webpack\src\angular_compiler_plugin.js:624:23)
    at plugin.done.then (C:\Users\Asad\dev\ngcli150\node_modules\@ngtools\webpack\src\loader.js:467:39)
    at <anonymous>
 @ ./src/app/app.component.ngfactory.js 9:0-141
 @ ./src/app/app.module.ngfactory.js
 @ ./src/main.ts
 @ multi webpack-dev-server/client?http://0.0.0.0:0 ./src/main.ts

Without AOT it works fine.

Note: I have to update typescript version to latest as well to make it work with latest angular and cli version.

Here is repo to reproduce this issue:

https://github.com/asadsahi/ngcli150

@asadsahi asadsahi changed the title SimpleNotificationsModule fails to load with angular-cli v1.5.0-rc8 SimpleNotificationsModule fails to load with angular-cli v1.5.0-rc8 under AOT Oct 31, 2017
@weihe8892
Copy link

weihe8892 commented Oct 31, 2017

I had same issue, the older version: 1.5.0-rc.3 is fine.

The build is also fine for 1.5.0-rc.8 if following files have been removed from node_modules/angular2-notifications/src/simple-notifications/components
notification.component.ngfactory.ts
simple-notifications.component.ngfactory.ts

@clydin
Copy link
Member

clydin commented Oct 31, 2017

The library should not have typescript source files present in its published package. It appears that the presence of an index.ts file in the root of the package is leading the compiler to use the TS source rather than the compiled version.

@asadsahi
Copy link
Author

@clydin do you know why it doesn't happen in non-aot mode?

Does that mean this issue needs to be raised with library owners?

@clydin
Copy link
Member

clydin commented Oct 31, 2017

At a minimum it should be raised with library owners. Even if there was not an underlying issue, the extra files are taking up hard drive space.

In addition there is potentially an issue with the priority of main file resolution of a module. The behavior here may be intended but further investigation is needed.

@clydin
Copy link
Member

clydin commented Oct 31, 2017

As an experiment, can you try manually removing the index.ts file from the package within node_modules?

@filipesilva
Copy link
Contributor

I saw something similar happen in #8216 (comment). I'm going to try to get more information about this, but it looks like the AOT compiler emiting JS for libraries that have TS files.

@filipesilva filipesilva self-assigned this Oct 31, 2017
@filipesilva filipesilva added P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix labels Oct 31, 2017
@asadsahi
Copy link
Author

@clydin getting more errors by removing index.ts:

c:/Users/Asad/ngnode-fullstack/src/app/core/core.module.ts
[0] src/app/core/core.module.ts(7,43): error TS7016: Could not find a declaration file for module 'angular2-notifications'. 'c:/Users/Asad/ngnode-fullstack/node_modules/angular2-notifications/dist/index.js' implicitly has an 'any' type.
[0]   Try `npm install @types/angular2-notifications` if it exists or add a new declaration (.d.ts) file containing `declare module 'angular2-notifications';`
[0] src/app/+login/reset-password/reset-password.component.ts(4,38): error TS7016: Could not find a declaration file for module 'angular2-notifications'. 'c:/Users/Asad/ngnode-fullstack/node_modules/angular2-notifications/dist/index.js' implicitly has an 'any' type.
[0]   Try `npm install @types/angular2-notifications` if it exists or add a new declaration (.d.ts) file containing `declare module 'angular2-notifications';`
[0] src/app/+profile/user-info/user-info.component.ts(6,38): error TS7016: Could not find a declaration file for module 'angular2-notifications'. 'c:/Users/Asad/ngnode-fullstack/node_modules/angular2-notifications/dist/index.js' implicitly has an 'any' type.
[0]   Try `npm install @types/angular2-notifications` if it exists or add a new declaration (.d.ts) file containing `declare module 'angular2-notifications';`
[0] src/app/+profile/user-photo/user-photo.component.ts(4,38): error TS7016: Could not find a declaration file for module 'angular2-notifications'. 'c:/Users/Asad/ngnode-fullstack/node_modules/angular2-notifications/dist/index.js' implicitly has an 'any' type.
[0]   Try `npm install @types/angular2-notifications` if it exists or add a new declaration (.d.ts) file containing `declare module 'angular2-notifications';`

@clydin
Copy link
Member

clydin commented Oct 31, 2017

The library's package.json is missing a typings field. It probably should be:

"typings": "./dist/index.d.ts",

@filipesilva
Copy link
Contributor

To give you all an update on the issue, @hansl is trying to work with the library authors of known problematic libraries to sort these issues out.

@karptonite
Copy link

karptonite commented Nov 1, 2017

@filipesilva Fixing the known problematic libraries is one important step, but if the breaking change that stops those libraries from working will remain, it would also be good if there were some sort of compiler warning (both for AOT and no-aot) when a library improperly includes TS files, so that unknown problematic libraries can be easily identified.

@filipesilva
Copy link
Contributor

Just wanted to link another related issue here; #8284.

The difference between these two issues that in that #8284 it was user code that was excluded, due to it not being listed in the tsconfig compilation files. In this issue, we're taking about vendor libraries.

I added a fix for #8284 in my comment there (#8284 (comment)). It might also work for cases like this, where you have an improperly packaged library, but I must stress that there is no guarantee that improperly packaged libraries will work.

@karptonite
Copy link

@filipesilva is there any guidance you can give on how to recommend that a library be packaged "properly"? Is there a simple test we can do so that we can submit a pull request to a given package, and be sure that the pull request will fix the issue?

@filipesilva
Copy link
Contributor

I'd really like to give you a single resource for that, but I don't have one. There's conversation on #1692 and #6510, with some alternative starters or library packages. I hope that helps.

@alan-agius4
Copy link
Collaborator

Closing as it looks like it's an issue on how a library is packaged.

@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 9, 2019
SuperButterfly pushed a commit to SuperButterfly/angular2-notifications that referenced this issue Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants