chrisguttandin / angular-prerender

A command line tool to prerender Angular Apps.
MIT License
126 stars 7 forks source link

Not picking up the routes array in angular.json #132

Closed psegarel closed 1 year ago

psegarel commented 1 year ago

Had to install it with --force in Angular 15, probably not related but angular-prerender is not picking up the routes in angular.json.

I can use the --include-routes but it gets fairly impractical with many routes. To simplify the task, I've added the command to the scripts in package.json, but I wouldn't like to add all the routes there.

There's probably a better way to do it but I'm not too familiar with node scripts & it would seem redundant since angular.json already gives the option

All those routes are dynamic routes with parameters, I wasn't able to use the --parameter-values flag, getting a number of errors related to the format of the json string

chrisguttandin commented 1 year ago

Hi @psegarel,

you're right Angular v15 is not yet officially supported. But as far as I know it's only a matter of updating the peer dependencies this time. I'll do it as soon as possible.

You said "angular-prerender is not picking up the routes in angular.json". Are you defining your routes inside of your angular.json? I had no idea that this is possible so far. Can you point me to some docs which describe how that works?

In case you have some index pages which link to all the routes with dynamic parameters using the --recursive flag might be an option.

psegarel commented 1 year ago

Hi @chrisguttandin,

Are you defining your routes inside of your angular.json? Some of them yes

I had no idea that this was possible so far. That's interesting. I really can't remember where I learnt that though. I couldn't find any docs. I will keep looking and will send you a link if I can find something.

In case you have some index pages which link to all the routes with dynamic parameters using the --recursive flag might be an option. I have tried that but I don't use routerLink much, making routes harder to guess I suppose

In these docs, I know that some of the parameters mentioned can be added in angular.json, under prerender like so:

"prerender": { "builder": "@nguniversal/builders:prerender", "options": { "guessRoutes": true, // default to false "routesFile": "routes.txt", "routes": [ "/", "/article/1", "/article/2", "/article/3" ] },

Please try it when you have time.

I'm not sure if it's possible to use "routesFile" & "routes" at the same time, but, it wouldn't matter much because, so far, I've only been able to get a maximum of 3 routes correctly rendered with metatags!

So far, the only solution I could come up with was to prerender the first 3 routes with a standard build, then use app-renderer for a single route each time, it's a bit cumbersome so I put it all in a bunch of scripts that I can call in a single command but it doesn't feel right!

I have also tried to use angular-prerender to render all the dynamic routes at once but I hit the same threshold of 3, all the routes get pre-rendered but only three of them have the correct metatags

If I'm correct & for some reason, there is a threshold of 3, it would be great to have the possibility to loop thru several routes with angular-prerender

chrisguttandin commented 1 year ago

Ah, I see. I forgot about the option to configure the prerender builder inside the angular.json. At the time the prerender builder was added to Angular itself it was basically a copy of this package. But angular-prerender is not reading the configuration meant for the prerender builder. Therefore it will also not pick up any routes defined in there.

angular-prerender definitely has no limit configured anywhere. It's strange that it only picks up 3 of your routes. angular-prerender (and the prerender builder) uses the guess-parser to discover routes automatically. The guess-parser looks for route definitions but it doesn't analyze the routerLink directive.

The --recursive flag only looks for <a/> tags in the rendered HTML. It should be completely independent of the way the routing works.

It's a bit hard for me to guess what's going wrong in your case. Is your project publicly available? If not could you try to create an example which reproduces the problem?

psegarel commented 1 year ago

angular-prerender has no limit configured anywhere. It's strange that it only picks up 3 of your routes. To be clear, the problem is only with the prerendering of the metatags. All pages are pre-rendered correctly, only three have the metatags updated correctly. I believe it is a bug.

The --recursive flag only looks for tags in the rendered HTML. It should be completely independent of the way the routing works. I don't use <a/>tags either, most of the routing comes from a function call, but discovering the routes is not the issue. Sorry, I probably wasn't very clear

It's a bit hard for me to guess what's going wrong in your case. Is your project publicly available? Thanks for caring! I should take the time to report that behaviour but I'm not too sure how since I need to call Firestore & I believe you need a minimal project to report a bug. I could try with a mock JSON API, I guess...

Yes, the project is online but most of the metatags should display correctly since I'm using this hack of prerendering the first 3 routes with a standard build, then prerendering the remaining routes, one by one, with angular-prerender

Here it is anyway insense.vn

chrisguttandin commented 1 year ago

Thanks for sharing the project. I like the scrolling effect on the projects page. I was hoping the source code is available publicly but I guess that's not the case.

When I browse the site I can see an error logged in the console: "ERROR TypeError: this.item.brands is undefined". The source code is minified but I guess this has something to do with the generation of the meta tags.

The minified code looks like this: void 0 !== typeof this.item.brands. I guess this is written originally like this undefined !== typeof this.item.brands. But that's always evaluating to true. I guess it's a typo and you wanted to check for 'undefined' (the string) instead.

Maybe changing all these checks in your ngOnInit() function solve the problem with the missing meta tags.

ngOnInit() {
    'undefined' !== typeof this.item.thumbnailUrl && (this.imgUrl = this.item.thumbnailUrl),
    'undefined' !== typeof this.item.title && (this.title = this.item.title),
    'undefined' !== typeof this.item.name && (this.title = this.item.name),
    'undefined' !== typeof this.item.tags && (this.tags = this.item.tags.join(', ')),
    'undefined' !== typeof this.item.brands && (this.tags = this.item.brands.join(', '))
}
psegarel commented 1 year ago

Thanks for taking the time to have a look.

At some point, I had an issue with an error where I had to make sure a value wasn't undefined, so yes, I need to do some cleaning :) I must have messed up some code while I was trying to debug the issue

I wish you were right about the metatags but I did some tests once with a very basic setup where I created the site structure & didn't really display anything. Just needed to get enough data from Firestore to create the metatags. Same error.

What's surprising is that this limit of 3 is consistent, it doesn't seem to depend on the content. I have tried changing the routes added in angular.json, and the metatags consistently break after 3 routes.

Originally, the site was built in Angular 14 & I did a total rewrite in Angular 15, hoping it would help... Nope...

The only thing that helped is angular-prerender as it allowed me to pre-render a single route without affecting the previous build.

Also, if the problem came from my code, I guess it would break whichever method I use. Why am I able to properly render all the routes separately but can't do the same when they're grouped in a text file or angular.json?

psegarel commented 1 year ago

https://github.com/psegarel/angular-metatags

Here's a public project I created to test the issue. Please have a look if you have time, not sure what I'm missing

psegarel commented 1 year ago

Problem solved! I may have been caught out by some caching issues. Thanks a lot for your support!

chrisguttandin commented 1 year ago

I'm happy to hear that you found and solved the problem. Thanks for reporting back.

I'm going to close this issue now. Feel free to open another one if you run into any other problems with angular-prerender.