search

ngMorocco / ngx-darija

Official Angular in Darija landing page containing all video sessions.
https://angularindarija.dev
56 stars 28 forks source link

feat: pre render dynamic video sessions routes as well #32

Closed Mubramaj closed 3 years ago

Mubramaj commented 3 years ago

I modified the prerendering so we render all the sessions pages.

The file that describes the dynamic routes is routes.txt and is generated automatically by a script I created.

The result is all the session static pages generated at build time:

Screenshot from 2021-03-20 23-41-12

Mubramaj commented 3 years ago

putting back this PR in draft because I spotted an issue. Going to https://deploy-preview-32--ngx-darija.netlify.app/sessions/rT0FUs7uUks the HTML loads from pre rendering but then Loading is displayed.

If one of you guys have a clue to what is happening here @chihab @ikourfaln

Mubramaj commented 3 years ago

@chihab I found the reason, it is linked to netlify. It lower case all the urls https://answers.netlify.com/t/my-url-paths-are-forced-into-lowercase/1659

If you for example try to directly access https://deploy-preview-32--ngx-darija.netlify.app/sessions/rT0FUs7uUks netlify will redirect to https://deploy-preview-32--ngx-darija.netlify.app/sessions/rt0fus7uuks which mess up the youtube video id that is case sensitive :s

Trying to find a solution if you have any idea let me know

Mubramaj commented 3 years ago

Ok it should be fixed. The solution I found for now is to generate a mapper between the lower case youtube video sessions to the real case sensitive video id.

I also added the handling of error if someone try to access a video session with a wrong ID

Screenshot from 2021-03-21 14-51-21

chihab commented 3 years ago

There are some indentations issues, before committing please make sure to sync with main to get lint-staged and prettier fix git hook executed.

I think generated id mapper file should not be in git (or at least not the ids).

As discussed, we'll later

Mubramaj commented 3 years ago

@chihab it should be good to go. You might notice that I wrapped each netlify function into a folder. This is the only way I found to zip the .json mapping file with the functions. Otherwise it was not included in the publish

Mubramaj commented 3 years ago

@chihab Just a suggestion but I think it can be cleaner to use squash and merge option for merging pull requests instead of merge this way we have only 1 commit per pull request in the main branch. What do you think ?