Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Skip fonts with a data: URL source from fontPreloading check #1186

Merged
merged 3 commits into from
Mar 30, 2021

Conversation

westonruter
Copy link
Member

Fixes #1185

@westonruter westonruter marked this pull request as ready for review March 30, 2021 18:04
Copy link
Collaborator

@sebastianbenz sebastianbenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a test here: https://github.com/ampproject/amp-toolbox/tree/main/packages/page-experience/test-data/checks/fontPreloading

You can then run npm run test:snapshot to generate a snapshot.

@westonruter
Copy link
Member Author

I added a test in e9159fb.

You can then run npm run test:snapshot to generate a snapshot.

Actually npm run test:optimizer:snapshot, right? In any case, it's not seeming to work for me. Even on main it's failing with a with a bunch of errors like:

image

I did do npm install. I'm on Node v14.16.0 (LTS).

@sebastianbenz
Copy link
Collaborator

You need to run it inside the page-experience directory. Sorry for not mentioning that.

@westonruter
Copy link
Member Author

OK, I'm now in packages/page-experience and I ran npm install. However:

$ npm run test:snapshot

> @ampproject/toolbox-page-experience@2.8.0-canary.14 test:snapshot …/amp-toolbox/packages/page-experience
> PAGE_EXPERIENCE_SNAPSHOT=true jest

sh: jest: command not found

Shouldn't jest be added as a dependency?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fontPreloading check erroneously advises to preload a font which is a data: URL
2 participants