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

cli: support catch all routes (i.e. to a custom 404 page) #31

Closed
Splaktar opened this issue Dec 16, 2019 · 19 comments
Closed

cli: support catch all routes (i.e. to a custom 404 page) #31

Splaktar opened this issue Dec 16, 2019 · 19 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request
Milestone

Comments

@Splaktar
Copy link
Contributor

Splaktar commented Dec 16, 2019

Steps to Reproduce

  1. ng add @scullyio/init
  2. npm i --save-dev fs-extra
  3. ng build
  4. npm run scully

Error

Cleaned up /Users/splaktar/Git/.../dist/static/ folder.
started servers in background
Invalid content-type.
Expected application/json but received text/html; charset=UTF-8
<repeated about 30 times>
Invalid content-type.
Expected application/json but received text/html; charset=UTF-8
Could not connect to server

The debugger indicates that this is caused by the following line

httpGetJson('http://localhost:1864/_pong', {suppressErrors: true})

That URL (http://localhost:1864/_pong) seems to be returning my Angular app (in a broken state) rather than some JSON content.

Root Cause

A route like {path: '**', component: PageNotFoundComponent} breaks the ability to run Scully. Removing it works around the problem, but breaks the app's custom 404 page.

@SanderElias
Copy link
Contributor

@Splaktar I'm unable to reproduce this, and would love to debug this together with you.
I have a faint idea what might be happening, but going over the code doesn't bring me any clue here.

@Splaktar
Copy link
Contributor Author

I restarted from scratch with 0.0.8 and I can still reproduce this.

@Splaktar
Copy link
Contributor Author

Splaktar commented Dec 18, 2019

Angular CLI: 9.0.0-rc.6
Node: 12.13.0
OS: darwin x64

Angular: 9.0.0-rc.6
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router, service-worker
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.6
@angular-devkit/build-angular     0.900.0-rc.6
@angular-devkit/build-optimizer   0.900.0-rc.6
@angular-devkit/build-webpack     0.900.0-rc.6
@angular-devkit/core              9.0.0-rc.6
@angular-devkit/schematics        9.0.0-rc.6
@angular/cdk                      9.0.0-rc.5
@angular/fire                     5.2.3
@angular/material                 9.0.0-rc.5
@ngtools/webpack                  9.0.0-rc.6
@schematics/angular               9.0.0-rc.6
@schematics/update                0.900.0-rc.6
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2

As you can see above, I didn't mention that my app is a PWA. I'm not sure if that breaks Scully, but I'll try to disable service-worker and see how that goes.

@Splaktar
Copy link
Contributor Author

However, I have SW disabled for dev builds like ng build, so it shouldn't be a problem here.

    ServiceWorkerModule.register('ngsw-worker.js', {
      enabled: environment.production,
      registrationStrategy: 'registerWithDelay:5000'
    }),

@Splaktar
Copy link
Contributor Author

Opening http://localhost:1864/ gives me a broken page with the following errors

Failed to load module script: The server responded with a non-JavaScript MIME type of "text/html". Strict MIME type checking is enforced for module scripts per HTML spec.
polyfills-es2015.js:1 Failed to load module script: The server responded with a non-JavaScript MIME type of "text/html". Strict MIME type checking is enforced for module scripts per HTML spec.
styles-es2015.js:1 Failed to load module script: The server responded with a non-JavaScript MIME type of "text/html". Strict MIME type checking is enforced for module scripts per HTML spec.
vendor-es2015.js:1 Failed to load module script: The server responded with a non-JavaScript MIME type of "text/html". Strict MIME type checking is enforced for module scripts per HTML spec.
main-es2015.js:1 Failed to load module script: The server responded with a non-JavaScript MIME type of "text/html". Strict MIME type checking is enforced for module scripts per HTML spec.
manifest.webmanifest:1 Manifest: Line: 1, column: 1, Syntax error.

For each of these failed resources, the server is sending back the contents of index.html instead of the proper file. This is likely happening with the _pong endpoint where it is getting the contents of index.html instead of a JSON payload.

@Hypercubed
Copy link

I see the same issue... nothing special in my test app. Generated a new ng8 app using the cli, ng updated to ng9-rc, then ng added scully.

Running npm run scully throws the Invalid content-type errors.

I see lots of js files in dist/static, however, all js requests on http://localhost:1864/ return contents of index.html.

@Splaktar
Copy link
Contributor Author

I wondering if there might be some dependance or issue with having Angular CLI version 8 installed globally, even if v9 is installed in the workspace. However, installing Angular CLI v9 globally didn't change anything.

I also tried ng build --prod instead of ng build, but that also had no effect.

@Hypercubed
Copy link

@Splaktar what routes do you have in your app?

I've think I have discovered if you try adding a route with only a slug (path: ":page"), I get the errors above. I assume because http://localhost:1864/_pong is being treated like a angular defined route.

Similarly, looks like http://localhost:1864/killMe doesn't work so the Scully server never shuts down and all subsequent runs of npm run scully will fail.

Kill the appropriate node process make all routes static (for the moment) and try again?

@Splaktar
Copy link
Contributor Author

export const AppRoutes: Routes = [
  {
    path: '',
    pathMatch: 'full',
    component: AboutComponent,
    children: []
  },
  {path: '**', component: PageNotFoundComponent}
];

@Splaktar
Copy link
Contributor Author

I've noticed that the node processes stay around and have to be manually killed, otherwise they sit on those two ports indefinitely.

@Splaktar
Copy link
Contributor Author

@Hypercubed this video covers how to set things up with the route parameters. However, I'm running into this issue w/o using any route params.

@Hypercubed
Copy link

Try removing the ** catch all....

@Splaktar
Copy link
Contributor Author

That worked (after killing the node process that was still around from the previous run)!

Cleaned up /Users/splaktar/Git/.../dist/static/ folder.
started servers in background
servers available
Finding all routes in application.
Pull in data to create additional routes.
Route list created in files:
      src/assets/scully-routes.json
      /Users/splaktar/Git/.../dist/static/assets/scully-routes.json
Route "" rendered into file: "/Users/splaktar/Git/.../dist/static/index.html"
Route list created in files:
      src/assets/scully-routes.json
      /Users/splaktar/Git/.../dist/static/assets/scully-routes.json

Generating took 9.32 seconds for 1 pages:
  That is 0.11 pages per second,
  or 9327 milliseconds for each page.

@Splaktar Splaktar changed the title Invalid content-type. Expected application/json but received text/html; charset=UTF-8 cli: support catch all routes (i.e. to a custom 404 page) Dec 19, 2019
@SanderElias SanderElias added bug Something isn't working enhancement New feature or request and removed need more info need repo example labels Dec 20, 2019
@SanderElias SanderElias self-assigned this Dec 20, 2019
@Hypercubed
Copy link

Hypercubed commented Dec 20, 2019

I'm pretty sure just need to move the _pong and killMe routes above the routes loop here:

routes.forEach(route => {

I haven't yet been able to build scully for dev so I can't test that.

Edit: PR submitted. Tested locally.

@SanderElias
Copy link
Contributor

@Hypercubed Yes, that is part of the issue, but it seems to go a bit deeper. I have a PR in flight that will address this. But as I'm officially on holiday, I'm not entirely sure when I will be able to complete this one. Might be next year ☺

@Hypercubed
Copy link

@SanderElias Are you able to share your PR? I'd like to test it if I can.

@jorgeucano
Copy link
Contributor

the fix is in master :)

@Splaktar
Copy link
Contributor Author

Splaktar commented Jan 9, 2020

Was this fixed in PR #146 or another PR?

@SanderElias
Copy link
Contributor

@Splaktar Yes, a partial fix might be in a bit earlier, but that one closed it completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
4 participants