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

With modified deploy-url, inlining critical css not working for downloaded roboto font #22208

Closed
1 of 15 tasks
school-coder opened this issue Nov 21, 2021 · 19 comments
Closed
1 of 15 tasks

Comments

@school-coder
Copy link

🐞 With modified deploy-url, inlining critical css not working for downloaded roboto font

Command (mark with an x)

  • new
  • build
  • serve
  • test
  • e2e
  • generate
  • add
  • update
  • lint
  • extract-i18n
  • run
  • config
  • help
  • version
  • doc

Is this a regression?

No.

In v12 also, was not working.

Description

A clear and concise description of the problem...

🔬 Minimal Reproduction

With modified deploy-url, inlining critical css not working for downloaded roboto font

package.json

"scripts": {
    ...
    "build": "node --max_old_space_size=5048 ./node_modules/@angular/cli/bin/ng build --base-href=/portal/ --deploy-url=/portal/public/ --configuration production"
    ...
}

font.css

@font-face {
  font-family: 'Roboto';
  src: url('Roboto-Light.woff2') format('woff2'),
  url('Roboto-Light.woff') format('woff');
  font-weight: 300;
  font-style: normal;
  font-display: swap;
}

angular.json
projects..architect.build:

...
"styles": [
             ...
              "src/assets/fonts/font.css",
             ...
],
...
"configurations": {
    "production": {
        ....
        "optimization": {
                        "scripts": true,
                        "styles": {
                          "minify": true,
                          "inlineCritical": true
                        }
                      }
        ....
    }
...
}

🔥 Exception or Error

image

the path is http://localhost:8080/portal/Roboto-Light.86fc2559ff73eac5.woff2 instead of http://localhost:8080/portal/public/Roboto-Light.86fc2559ff73eac5.woff2

🌍 Your Environment

13.0.0

@alan-agius4
Copy link
Collaborator

Can you please provide the values of deployUrl and baseHref options?

@school-coder
Copy link
Author

school-coder commented Nov 22, 2021

Can you please provide the values of deployUrl and baseHref options?

@alan-agius4, Yes I have mentioned it in the description under package.json

"scripts": {
    ...
    "build": "node --max_old_space_size=5048 ./node_modules/@angular/cli/bin/ng build --base-href=/portal/ --deploy-url=/portal/public/ --configuration production"
    ...
}

Observation here is, when I set inlineCritical to false, the URL of the font files are resolved correctly. But when I set this to true, font files are not loading due to incorrect URL.

@alan-agius4
Copy link
Collaborator

Hi @school-coder,

I managed to replicate this. However, it's important to point out the deployUrl is deprecated and will be removed in a future version of the Angular CLI.

Since this is not a regression I am inclined to say that we shouldn't be fixing deprecated behaviour unless it's a regression especially considering that this issue is not a problem when migrate to the recommended alternative for deployUrl.

@school-coder
Copy link
Author

@alan-agius4 : I am still not clear why deployUrl is been deprecated.

In my case, static assets needs to be load from /portal/public/<asset_path> where as routing or http request needs to fire with base-href which is /portal/.

In our case, we are doing white-listing the security for the path having public in it as it has only static assets.

What would be your recommendation in this case?

Note: I am also following the issue #22113 on the same usecase

@alan-agius4
Copy link
Collaborator

alan-agius4 commented Nov 22, 2021

@school-coder, You don't need deployUrl to achieve the above.

You can set the baseHref to /portal/public/ and APP_BASE_HREF to /portal/.

More info about the deprecation can be found here: #21537.

@school-coder
Copy link
Author

@alan-agius4, baseHref and APP_BASE_HREF does the same job right?

I tried the steps you have followed which resulted in,

<base href="/portal/public/">
....
<script src="runtime.24065062ffd17f11.js" type="module"></script>

generated in index.html. The scripts are resolving whereas http requests are misleading

For example,

Expected:

http://localhost:8080/portal/rest/v1/themes/styles

But the request triggered as,

http://localhost:8080/portal/public/rest/v1/themes/styles

@alan-agius4
Copy link
Collaborator

@alan-agius4, baseHref and APP_BASE_HREF does the same job right?

No they are 2 different settings although when APP_BASE_HREF is not specified it defaults to baseHref. The APP_BASE_HREF is used to set the router base href. See: https://angular.io/guide/deployment#the-deploy-url

The APP_BASE_HREF, doesn't effect the HTTP Client. And hence, the "incorrect" HTTP request URL. Using an HTTP interceptor you should be able to amend the request Url quite easily.

@mpalourdio
Copy link

mpalourdio commented Nov 22, 2021

Yes, that's exatcly what I have experienced when trying to use APP_BASE_HREF to get rid of deployUrl. The HTTP requests are relative to baseHref, not to APP_BASE_HREF.

I have ended up with an interceptor, but not 100% satisfying IMO.

import { HTTP_INTERCEPTORS, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http';
import { ExistingProvider, Injectable } from '@angular/core';
import { Observable } from 'rxjs';

@Injectable({
    providedIn: 'root'
})
export class BasehrefInterceptor implements HttpInterceptor {

    intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
        if (!req.url.toLocaleLowerCase().startsWith('http')) {
            const dupReq = req.clone({ url: `../${req.url}` });
            return next.handle(dupReq);
        }
        return next.handle(req);
    }
}

export const BasehrefInterceptorProvider: ExistingProvider[] = [{
    provide: HTTP_INTERCEPTORS,
    useExisting: BasehrefInterceptor,
    multi: true
}]

@school-coder
Copy link
Author

@mpalourdio +1

@mpalourdio
Copy link

mpalourdio commented Nov 22, 2021

Also there's a problem I think when using the dev server. (I was just testing in build prod mode for the moment)

Imagine you configure this in dev mode to handle this use case.

"baseHref": "/my-context/path/static/",

and this, as @alan-agius4 said it's supposed to work now:
providers: [ { provide: APP_BASE_HREF, useValue: '/my-context/path/' }, ]

Now, go to http://localhost:4200/my-context/path/ => 404. That's wrong, as it's supposed to be the router base as stated by the APP_BASE_HREF documention.

Ok, so let's go to http://localhost:4200/my-context/path/static/ : redirects to http://localhost:4200/my-context/path/ and it works this time...

Refresh the page : 404

@school-coder
Copy link
Author

Also there's a problem I think when using the dev server. (I was just testing in build prod mode for the moment)

Imagine you configure this in dev mode to handle this use case.

"baseHref": "/my-context/path/static/",

and this, as @alan-agius4 said it's supposed to work now: providers: [ { provide: APP_BASE_HREF, useValue: '/my-context/path/' }, ]

Now, go to http://localhost:4200/my-context/path/ => 404. That's wrong, as it's supposed to be the router base as stated by the APP_BASE_HREF documention.

Ok, so let's go to http://localhost:4200/my-context/path/static/ : redirects to http://localhost:4200/my-context/path/ and it works this time...

Refresh the page : 404

Exactly, facing the same with the dev server after following the suggestion.

@alan-agius4 : IMO, the alternative you have provided doesnt solve the use-case.

@alan-agius4
Copy link
Collaborator

alan-agius4 commented Nov 22, 2021

@school-coder, currently, what are the deployUrl and baseHref configuration that you use for the dev-server?

Let me explain why the above errors and why they are happening:

  • http://localhost:4200/my-context/path/, the dev-server doesn't know how to handle this, since this is just a client side route hence it's a 404.
  • http://localhost:4200/my-context/path/static/, when accessing this page Angular will bootstrap and redirect to the defined APP_BASE_HREF, ie: http://localhost:4200/my-context/path/, you can navigate through the app normally.
  • Refreshing the page will cause an HTTP GET that the server doesn't know how to handle since routing is managed by Angular and hence 404.

During development deployUrl and baseHref cannot be both absolute, therefore with the new setup you need to mimic that setup, ie by removing APP_BASE_HREF during development.

The dev-server, is a minimal server that doesn't offer all the functionality that a production server has.

@mpalourdio
Copy link

mpalourdio commented Nov 22, 2021

Yes, I understand why those 404 happen, of course. But, when removing deployUrl in favor of baseHref + APP_BASE_HREF, why would one remove APP_BASE_HREF in dev mode ? I don't get it.

And as you can see in my example, both baseHref and APP_BASE_HREF are relative (and deployUrl is removed from the project).

Also navigating to http://localhost:4200/my-context/path/static/ in order to be redirecting to http://localhost:4200/my-context/path/ is counter intuitive. As soon as you modify the code, and the live-reload kicks in, you're screwed :/

@alan-agius4
Copy link
Collaborator

During development you can also set, APP_BASE_HREF, baseHref and servePath to the same value.

Some of the drawbacks of deployUrl

  • The deployUrl value is scattered all over the bundles, which causes an increase in size.
  • It effects only assets referenced in CSS files, which makes the behaviour of assets non consistent when they are referenced in HTML templates.
  • Slower builds, due to extra CSS processing.

@school-coder
Copy link
Author

school-coder commented Nov 22, 2021

@mpalourdio and @alan-agius4 : I have couple of updates

Update 1: Adaption of suggestion is now working

My Environment:

baseHref='/portal/public/'
APP_BASE_HREF = '/portal/'

Adaption i have done,

@Injectable()
export class RequestInterceptor implements HttpInterceptor {

  constructor(private store: Store<UIState>, @Inject(APP_BASE_HREF) private baseHref:string) {
  }

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const newRequest = req.clone({ url: this.baseHref + req.url });
    return next.handle(newRequest);
  }
}


Dev Server and Production Server working fine now. I need not to do any change specific to dev server. For example removing APP_BASE_HREF, Only thing, I need to adapt static urls used through out the application.

for example,

I need to adapt the url public/assets/images/logo.svg to assets/images/logo.svg.

Update 2: Enabling inlineCritical generates right URL now.

After configurating inlineCritical as true, the URL is generating correctly which is the actual issue I reported.

Update 3: Content Security Policy Issue while enabling inlineCritical.

image

I doubt it is related to the generated link element,

<link rel="stylesheet" href="styles.870562dd6d1ef9f0.css" media="print" onload="this.media='all'">

As this link element is generated during the build, adding hash or nonce attribute can not be done.

Is there anyway to resolve this?

My Current Content Security Policy,

content-security-policy: default-src 'self'; img-src * 'self' data:; object-src 'none'; child-src * mailto: tel: ms-word:; script-src 'self'; style-src 'self' 'unsafe-inline'; form-action 'self' *.okta.com; frame-ancestors 'self';

@alan-agius4
Copy link
Collaborator

The CSP issue is a known limitation which is being tracked here: #20864

@mpalourdio
Copy link

mpalourdio commented Nov 22, 2021

@alan-agius4 @school-coder I think I have made it work as desired too (The interceptor is indeed really needed, and injecting the APP_BASE_HREF was a good idea!), thanks both for your valuable feedback.

I suspect this will pop again and again on this tracker once deployUrl is removed, because devs are lazy by design, and a deprecation warning is something our eyes ignore until the feature is definitely removed :)

Once again, thanks ! And keep-up the good work angular team, you do an awesome job!

@alan-agius4 alan-agius4 self-assigned this Nov 29, 2021
@alan-agius4
Copy link
Collaborator

Glad to hear that the issue is solved by not using deployUrl.

@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 Dec 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants