-
Notifications
You must be signed in to change notification settings - Fork 16
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
[OPTIMISATION] fetch platform specific chrome image #12
base: main
Are you sure you want to change the base?
Conversation
Someone is attempting to deploy a commit to a Personal Account owned by @vikiival on Vercel. @vikiival first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hey thanks for the PR! I am currently away from Github, will get back to this PR next week |
Hey @vikiival ! Were you able to take a look at this? |
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.
Looks okish, just small things
args: [...chrome.args, '--hide-scrollbars', '--disable-web-security'], | ||
defaultViewport: chrome.defaultViewport, | ||
executablePath: await chrome.executablePath( | ||
`https://github.com/Sparticuz/chromium/releases/download/v116.0.0/chromium-v116.0.0-pack.tar` |
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.
prob path should be optional?
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.
There are 3 ways to do this:
- Keep a chromium tar file in code and supply that as an executable. Caveat: increases bundle size.
- Store the chromium tar in personal S3 bucket or something similar.
- Use github tar file
Either ways we need to provide an executable in Cloud Environment.
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 was an original code that worked on Vercel
executablePath: await chrome.executablePath(),
process.platform === "win32" | ||
? "C:\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe" | ||
: process.platform === "linux" | ||
? "/usr/bin/google-chrome" | ||
: "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"; |
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.
not a fan of double ternary ;D
code works w/ different environments now