Skip to content

Feature to ignore specific ResourceTypes in Puppeteer#589

Merged
SanderElias merged 1 commit intoscullyio:masterfrom
samvloeberghs:master
Jun 1, 2020
Merged

Feature to ignore specific ResourceTypes in Puppeteer#589
SanderElias merged 1 commit intoscullyio:masterfrom
samvloeberghs:master

Conversation

@samvloeberghs
Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Other... Please describe:

What is the current behavior?

All images / fonts / stylesheets are currently also loaded in Puppeteer when requesting a page. In most cases they are not relevant for prerendering the HTML.

What is the new behavior?

Any ResourceType that is listed here will be ignored by the Puppeteer instance rendering the requested page. For example if you add image and fonts all requests to images and fonts loaded on your pages will be blocked.

Does this PR introduce a breaking change?

  • Yes
  • No (it's opt-in)

Other information

@samvloeberghs
Copy link
Copy Markdown
Contributor Author

Can we first validate the change? If all good I will add tests where applicable!

@samvloeberghs
Copy link
Copy Markdown
Contributor Author

Can we first validate the change? If all good I will add tests where applicable!

Can't seem to find any case where I should update / add tests. Is that correct?

@samvloeberghs samvloeberghs marked this pull request as ready for review May 27, 2020 20:25
@jorgeucano jorgeucano requested a review from SanderElias May 28, 2020 13:13
@SanderElias
Copy link
Copy Markdown
Contributor

@samvloeberghs the tests are in the /test folder. We have jest and cypress e2e tests in there.
They there Scully as a whole, not the individual parts.

@samvloeberghs
Copy link
Copy Markdown
Contributor Author

samvloeberghs commented May 29, 2020

@SanderElias
I have ran the tests like this and did not experience issues:

  1. Did a first check of the configuration / process.
    Running Scully with the sampleBlog.config generated output in dist/static/.
    Running the tests with jest yielded no errors
npm i
npx nx build scully --prod
npx nx build ng-lib --prod
npx nx build scully-plugin-flash-prevention --prod
npm run symlinks
npx nx build sample-blog --prod
node ./dist/libs/scully/scully --configFile scully.sampleBlog.config.js --tds
npm run jest:test
  1. Updated the scully.sampleBlog.config.js to include the to ignore ResourceTypes:
  ignoreResourceTypes: [
    'image',
    'stylesheet',
    'media',
    'font'
  ]
  1. Redid the testing process, no errors, so the output is the same!
node ./dist/libs/scully/scully --configFile scully.sampleBlog.config.js --tds
npm run jest:test

@aaronfrost
Copy link
Copy Markdown
Contributor

@samvloeberghs after the testing... any chance that you noticed an improvement in speed? If so, can you comment?

@samvloeberghs
Copy link
Copy Markdown
Contributor Author

In the order of 100 pages there was no performance improvement. I would need to run the tests for 1000 and 10000 pages.

But the amount of bandwidth potentially saved is enormous in those test cases.

@SanderElias SanderElias merged commit d73388f into scullyio:master Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants