-
Notifications
You must be signed in to change notification settings - Fork 83
feat: make build-info browser compatible #4869
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
Conversation
|
This pull request adds or modifies JavaScript ( |
danez
left a comment
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 really good.
|
|
||
| join(...segments: string[]): string { | ||
| return join(...segments) | ||
| } |
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.
Just noting that path is easy to polyfill in the browser, so we do not necessarily need to add it to the FileSystem here, but I'm also not against doing that.
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.
But then you would need webpack ;) and I've built an implementation for it (added good tests as well)
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes https://github.com/netlify/pillar-workflow/issues/1011
Optimisations on the WebFS where not done (to reduce http calls)
I will do this in a follow up PR as this is already a huger PR size.
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)