gonzofish / angular-librarian

An Angular 2+ scaffolding setup for creating libraries
https://www.npmjs.com/package/angular-librarian
MIT License
91 stars 9 forks source link

Build breaks after upgrade to angular 5 #78

Closed tommueller closed 6 years ago

tommueller commented 6 years ago

I cannot build my library anymore after upgrading to angular 5. I breaks with the following message:

TypeError: args.indexOf is not a function at module.exports (/home/tom/workspace-angular/ng-core/node_modules/minimist/index.js:44:14) at readNgcCommandLineAndConfiguration (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/main.js:66:41) at main (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/main.js:21:24) at Promise.all.map (/home/tom/workspace-angular/ng-core/tasks/build.js:35:5) at Array.map (native) at compileCode (/home/tom/workspace-angular/ng-core/tasks/build.js:34:49) at /home/tom/workspace-angular/ng-core/tasks/build.js:26:16

Any ideas on how to fix this?

gonzofish commented 6 years ago

are you publishing a scoped package?

what versions of Node & NPM are you using?

tommueller commented 6 years ago

yes it is scoped.

npm --version 3.10.10

node --version

v6.11.5

it breaks in the call of ngc though, because it cannot read the args using minimist ... I tried to understand it, but was lost pretty fast ;)

gonzofish commented 6 years ago

ok, there seems to be a bug with scoped packages in NPM 5, which is why I asked--but you're using 3 so its a moot point. I'll try my best to figure it out.

gonzofish commented 6 years ago

Btw, any idea what the file src/main.js could be?

tommueller commented 6 years ago

Ah yes, sorry. Quoting in github removed it because of the '@' before angular. It is in the compiler-cli

at main (/home/tom/workspace-angular/ng-core/node_modules/'@'angular/compiler-cli/src/main.js:32:139)

gonzofish commented 6 years ago

Ah, ok, perhaps the CLI changed in 5? I haven't worked with 5, nor is Librarian v5-tested, so I can't speak to what's going on there

tommueller commented 6 years ago

Yes, I upgraded to v5 and isung compiler-cli@5.0.0

I could not find any documentation on how to use the compiler-cli from code ...

Do you have any plans on diving into this? Otherwise I will try to find out stuff.

gonzofish commented 6 years ago

I do have plans to, but I'm going to get a v1 of Librarian released first with what exists for now and that will be part of a 1.1 release

On Thu, Nov 9, 2017 at 9:27 AM Tom Müller notifications@github.com wrote:

Yes, I upgraded to v5 and isung compiler-cli@5.0.0

I could not find any documentation on how to use the compiler-cli from code ...

Do you have any plans on diving into this? Otherwise I will try to find out stuff.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-343170471, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYjrhAGsCncxLpcsE7WSF4vOhztfznkks5s0wvbgaJpZM4QU2Dm .

tommueller commented 6 years ago

Ok, I am not really in hurry. If I find time, I will try to find some info and post it here.

Cheers! Tom

tommueller commented 6 years ago

Well I figured out that it seems to breaking in this line

ngc({ project: path.resolve(rootDir, 'tsconfig.es${ type }.json')})

because ngc now takes 3 arguments args, consoleError, config now treats this config as args, which cannot be parsed by minimist since it is not a string object.

I tried playing around with calling something like:

ngc(null, null, { project: path.resolve(rootDir, 'tsconfig.es${ type }.json')})

to tell ngc to use this as config, but then other config options are missing, I also tried:

ngc('--project ' + path.resolve(rootDir, 'tsconfig.es${ type }.json'))

to run into the parsing of args, but that did not work either because minimist wasn't willing to parse that correctly.

So I don't really know what to do from here ...

gonzofish commented 6 years ago

At first look, it seems that there isn't much of a change.

Have you tried passing as an array? Like:

ngc(['--project', path.resolve(rootDir, `tsconfig.es${ type }.json`)], null)

From the source args is string[]

gonzofish commented 6 years ago

You should also be able to pass in an empty array for args--I think

litzebauer commented 6 years ago

@gonzofish Its a little more than that because it doesn't return a promise anymore either. I should have a fix soon

gonzofish commented 6 years ago

@tommueller according to @litzebauer your fix should be available on master

If you want to test-run it, point Librarian to git+https://github.com/gonzofish/angular-librarian.git in your package.json and run ngl up

If it works for you, you can close the issue

litzebauer commented 6 years ago

My PR allows the build to succeed but now consuming that library from another angular-librarian library is resulting in Please add a @NgModule annotation. errors

@tommueller Have you had any luck?

tommueller commented 6 years ago

No, sorry. Will also probably not have time to look into this until Wednesday. Thanks for almost fixing it ;)

litzebauer commented 6 years ago

Looks like index.js and a metadata file for index is not generated in the dist directory. There's just a index.ts file. I wonder if this is the problem. In my project using an older angular-librarian there is an index.js and index.metadata.json.

gonzofish commented 6 years ago

Good catch, I'll try to look into this weekend if you don't figure it out

On Fri, Nov 10, 2017 at 3:29 PM Jeremy Litzebauer notifications@github.com wrote:

Looks like index.js and a metadata file for index is not generated in the dist directory. There's just a index.ts file. I wonder if this is the problem. In my project using an older angular-librarian there is an index.js and index.metadata.json.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-343577900, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYjrmiIXcVKUkP6uv_7d3oE4nOarmGJks5s1LI9gaJpZM4QU2Dm .

litzebauer commented 6 years ago

Thanks @gonzofish Here's what I've looked into:

Looks like this last issue doesn't have to do with Angular 5 and has something to do with one of the commits made to master since the last release.

litzebauer commented 6 years ago

@gonzofish Did you get a chance to look at this at all?

gonzofish commented 6 years ago

I haven't been able to--new job started today and I had a busy weekend. I'll try to get to it sometime this week

litzebauer commented 6 years ago

I fixed this last issue. Typescript was set to ^2.4.0 which caused 2.6.x to be pulled in causing decorators not to be written out. Angular 5 migration guide says set Typescript to 2.4.2 which is what I did and the decorators are back. @tommueller can you close this issue once you give it a try?

gonzofish commented 6 years ago

Excellent ! Thanks for fixing it On Tue, Nov 14, 2017 at 2:47 PM Jeremy Litzebauer notifications@github.com wrote:

I fixed this last issue. Typescript was set to ^2.4.0 which caused 2.6.x to be pulled in causing decorators not to be written out. Angular 5 migration guide says set Typescript to 2.4.2 which is what I did and the decorators are back. @tommueller https://github.com/tommueller can you close this issue once you give it a try?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-344375883, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYjrl19w5mbSWP_fkUxmd6gFis0Z_Lwks5s2e5LgaJpZM4QU2Dm .

tommueller commented 6 years ago

@litzebauer that's great news! will give it a try tomorrow and report back! thanks for the work!

tommueller commented 6 years ago

Sorry took a day longer, but now I am on it. So far it was not straight forward to get it working, because after running ngl upgrade the merging of the two package.json-files gave me a lots of broken peerDependencies, as I had a funny mix of @angular ^4.0.0 and ^5.0.0 versions.

After fixing this it now compiles but breaks with an error, that I did not get before ...

[Librarian]: Error: Error: /home/tom/workspace-angular/ng-core/build/util/js.utils.ts:13:1: Error encountered in metadata generated for exported symbol 'JsUtils': /home/tom/workspace-angular/ng-core/build/util/js.utils.ts:24:39: Metadata collected contains an error that will be reported at runtime: Expression form not supported.

The reported function looks like this:

export class JsUtils {    
    public static escapeHtml(source: string): string {
        return String(source).replace(/[&<>"'\/]/g, (s) => entityMap[s]);
    }
}

any ideas what makes this fail now?

litzebauer commented 6 years ago

@tommueller I haven't seen that issue. Is that being thrown from angular-cli?

tommueller commented 6 years ago

This is the whole error-stack:

Error: Error: /home/tom/workspace-angular/ng-core/build/util/js.utils.ts:13:1: Error encountered in metadata generated for exported symbol 'JsUtils':
 /home/tom/workspace-angular/ng-core/build/util/js.utils.ts:24:39: Metadata collected contains an error that will be reported at runtime: Expression form not supported.
  {"__symbolic":"error","message":"Expression form not supported","line":23,"character":38}
    at /home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/metadata/collector.js:663:27
    at Array.forEach (native)
    at validateMetadata (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/metadata/collector.js:651:42)
    at MetadataCollector.getMetadata (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/metadata/collector.js:508:17)
    at LowerMetadataCache.getMetadataAndRequests (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:263:39)
    at LowerMetadataCache.ensureMetadataAndRequests (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:209:27)
    at LowerMetadataCache.getRequests (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:204:21)
    at /home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:146:36
    at /home/tom/workspace-angular/ng-core/node_modules/typescript/lib/typescript.js:2492:86
    at reduceLeft (/home/tom/workspace-angular/ng-core/node_modules/typescript/lib/typescript.js:2188:30)
litzebauer commented 6 years ago

Is this in your library when running npm run build?

On Thu, Nov 16, 2017 at 9:25 AM Tom Müller notifications@github.com wrote:

This is the whole error-stack:

Error: Error: /home/tom/workspace-angular/ng-core/build/util/js.utils.ts:13:1: Error encountered in metadata generated for exported symbol 'JsUtils': /home/tom/workspace-angular/ng-core/build/util/js.utils.

ts:24:39: Metadata collected contains an error that will be reported at runtime: Expression form not supported. {"__symbolic":"error","message":"Expression form not supported","line":23,"character":38} at /home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/metadata/collector.js:663:27 at Array.forEach (native) at validateMetadata (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/metadata/collector.js:651:42) at MetadataCollector.getMetadata (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/metadata/collector.js:508:17) at LowerMetadataCache.getMetadataAndRequests (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:263:39) at LowerMetadataCache.ensureMetadataAndRequests (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:209:27) at LowerMetadataCache.getRequests (/home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:204:21) at /home/tom/workspace-angular/ng-core/node_modules/@angular/compiler-cli/src/transformers/lower_expressions.js:146:36 at /home/tom/workspace-angular/ng-core/node_modules/typescript/lib/typescript.js:2492:86 at reduceLeft (/home/tom/workspace-angular/ng-core/node_modules/typescript/lib/typescript.js:2188:30)

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-344937881, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLc63itk3zwWcuOCNYnvssxNYLbHR-cks5s3EX0gaJpZM4QU2Dm .

tommueller commented 6 years ago

@litzebauer ngl build and npm run build result in the exact same error and output.

tommueller commented 6 years ago

Seems to be an error in the compiler-cli that is supposed to be fixed ... https://github.com/angular/angular/issues/18867

tommueller commented 6 years ago

Okay, the problem is really in the specific HTML-escape function, removing it makes the build run through succesfully! I will close this then. Will there be a new release incorporating this soon?

gonzofish commented 6 years ago

I'll try to get a 1.0.0 release out the door soon

tommueller commented 6 years ago

sorry for being all over you all the time @gonzofish but is there an eta on the 1.0.0? would love to finally "officially" upgrade to angular 5.

Cheers!

sameh-amin commented 6 years ago

yes please, i have already upgraded to angular 5 and i am stucked now.

Thanks :)

gonzofish commented 6 years ago

Hey, sorry for the delay--I haven't had time to put the 1.0 delivery together yet. I need to do some testing & upgrading to 5 before I can release it. Again, sorry for the delay!

gonzofish commented 6 years ago

Just an FYI, I'm trying to get this 1.0.0 done but I'm running into major issues with publish...np just seems to not want to let tasks finish before it thinks its done

gonzofish commented 6 years ago

@tommueller I was able to get np working but it introduces another, potentially more concerning problem...

Angular 5 wants RxJS ^5.5.0 NP can't work with any version past 5.4.3

Angular 5 will work with 5.4.3 but every time you install you'll get warnings about not having the right peerDependency installed...I'm not really sure what to do here, maybe @litzebauer has an opinion?

litzebauer commented 6 years ago

Why can't np work with rxjs. Since np hasn't be working in angular-librarian, I installed it globally and it had been working fine.

On Sun, Dec 17, 2017, 10:03 PM Matt Fehskens notifications@github.com wrote:

@tommueller https://github.com/tommueller I was able to get np working but it introduces another, potentially more concerning problem...

Angular 5 wants RxJS ^5.5.0 NP can't work with any version past 5.4.3

Angular 5 will work with 5.4.3 but every time you install you'll get warnings about not having the right peerDependency installed...I'm not really sure what to do here, maybe @litzebauer https://github.com/litzebauer has an opinion?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-352314367, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLc63XbDG8p8732-uYHN-yjADvhAX2rks5tBdX9gaJpZM4QU2Dm .

gonzofish commented 6 years ago

np wants 5.4.3. Anything past that breaks how tasks are run in np. The reason it works globally is because the global and your library’s versions of RxJS are separate. I suppose we can make a global install of np be the new standard for librarian? On Sun, Dec 17, 2017 at 10:18 PM Jeremy Litzebauer notifications@github.com wrote:

Why can't np work with rxjs. Since np hasn't be working in angular-librarian, I installed it globally and it had been working fine.

On Sun, Dec 17, 2017, 10:03 PM Matt Fehskens notifications@github.com wrote:

@tommueller https://github.com/tommueller I was able to get np working but it introduces another, potentially more concerning problem...

Angular 5 wants RxJS ^5.5.0 NP can't work with any version past 5.4.3

Angular 5 will work with 5.4.3 but every time you install you'll get warnings about not having the right peerDependency installed...I'm not really sure what to do here, maybe @litzebauer https://github.com/litzebauer has an opinion?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub < https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-352314367 , or mute the thread < https://github.com/notifications/unsubscribe-auth/ABLc63XbDG8p8732-uYHN-yjADvhAX2rks5tBdX9gaJpZM4QU2Dm

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-352316024, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYjrl7As9kKlH5iMX8sj1yuaC-EFkafks5tBdmRgaJpZM4QU2Dm .

litzebauer commented 6 years ago

Yeah I guess that would be fine.

On Sun, Dec 17, 2017 at 10:21 PM Matt Fehskens notifications@github.com wrote:

np wants 5.4.3. Anything past that breaks how tasks are run in np. The reason it works globally is because the global and your library’s versions of RxJS are separate. I suppose we can make a global install of np be the new standard for librarian? On Sun, Dec 17, 2017 at 10:18 PM Jeremy Litzebauer < notifications@github.com> wrote:

Why can't np work with rxjs. Since np hasn't be working in angular-librarian, I installed it globally and it had been working fine.

On Sun, Dec 17, 2017, 10:03 PM Matt Fehskens notifications@github.com wrote:

@tommueller https://github.com/tommueller I was able to get np working but it introduces another, potentially more concerning problem...

Angular 5 wants RxJS ^5.5.0 NP can't work with any version past 5.4.3

Angular 5 will work with 5.4.3 but every time you install you'll get warnings about not having the right peerDependency installed...I'm not really sure what to do here, maybe @litzebauer https://github.com/litzebauer has an opinion?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub <

https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-352314367

, or mute the thread <

https://github.com/notifications/unsubscribe-auth/ABLc63XbDG8p8732-uYHN-yjADvhAX2rks5tBdX9gaJpZM4QU2Dm

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-352316024 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AAYjrl7As9kKlH5iMX8sj1yuaC-EFkafks5tBdmRgaJpZM4QU2Dm

.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/gonzofish/angular-librarian/issues/78#issuecomment-352316302, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLc6xf7E0yjuWkjj98nQA6pyBdKixJiks5tBdo2gaJpZM4QU2Dm .