-
Notifications
You must be signed in to change notification settings - Fork 4
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
Prerendering #10
base: master
Are you sure you want to change the base?
Prerendering #10
Conversation
const url = 'http://localhost:3000' | ||
|
||
if (!url) { | ||
throw "Please provide URL as a first argument"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this url is now hardcoded, therefore this if is unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True!
const browser = await puppeteer.launch(); | ||
const page = await browser.newPage(); | ||
await page.goto(url, {waitUntil: 'networkidle0'}); | ||
await page.screenshot({path: 'screenshot.png'}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this screenshot used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! I think it was used merely to test this beast. Not required anymore.
with: | ||
python-version: 3.8 | ||
|
||
- name: Run Prerender |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be ran on the branch the build was executed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True
set -x | ||
npm install | ||
npm start & | ||
./node_modules/.bin/wait-on http://localhost:3000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be divided into build.sh and generate-static-page.sh.
Or smth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or the entire app code could be rewritten but that's beyond my humble capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code is ok, however some changes should be done to preserve quality
No description provided.