-
Notifications
You must be signed in to change notification settings - Fork 324
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 #518
base: primary
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
.env.example
Outdated
@@ -0,0 +1 @@ | |||
NODE_ENV=development |
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.
Should we make development
the default and change the conditions to detect the production
environment instead? Then we wouldn't need the env file locally.
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.
Done. That's a nice catch since there are no other envs we can default to dev.
@@ -252,6 +252,13 @@ npm run dev | |||
<sub><b>Gonçalo Morais</b></sub> | |||
</a> | |||
</td> | |||
<td align="center"> |
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.
api/screenshot.js
Outdated
console.log('exePath:', exePath); | ||
if (isDev) { | ||
options = { | ||
args: [], |
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.
Is there a reason why args
and defaultViewport
are locally different than in prod?
Ahh. I see — because chromium
isn't available. Could we still add --hide-scrollbars
and --disable-web-security
? 🤔
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.
done
@realityzero Welcome to TH and thank you so much for contributing. 💯 This topic has been on the list for ages so I'm exited that someone is tackling it. I've left some comments and on the way and will test is locally once we discussed these. Thanks again and have a great day! |
woahh @stefanjudis this made my day |
@realityzero I just checked out the branch and have run Unfortunately, it's not working locally and I'm running in Error log
What's your local setup to test this? |
@stefanjudis Apologies for getting onto this quite late. I couldn't test the solution end to end locally from That's my bad. Could you please try this out: Stackoverflow Or just tell me how to invoke |
No worries. I'm still very grateful that you're taking this on! 🎉
That's my bad. As this project is deployed to vercel I run it via the CLI which I have installed globally.
I also looked briefly into this and no SO magic helped out for the local screenshoting. :/ |
@stefanjudis
Ref: |
@realityzero Unfortunately, neither of the options work. I'm still running into the same exceptions. Did you try the Vercel CLI to see how to tackle it? |
Hey @stefanjudis ! Apologies man for such a late reply. Certain life challenges. I did some debugging locally in two different stages.
I simply executed How about we get over a call for this in the coming week? |
@realityzero Hey, so sorry for letting this slip through. :/ Life's pretty wild on my end. But in the meantime, something changed on vercel's side and things broke entirely. I think I'll move the screenshots off serverless fns. It's just to briddle. ;/ Sorry for wasting your time. |
desc: Environment specific chromium image selection on development environment