-
Notifications
You must be signed in to change notification settings - Fork 294
fix: remove node.js dependency in browser #1081
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
fix: remove node.js dependency in browser #1081
Conversation
Visit the preview URL for this PR (updated for commit 2e73145):
https://docusaurus-openapi-36b86--pr1081-lq3u8386.web.app
(expires 2025年3月13日 18:43:09 GMT)
🔥 via Firebase Hosting GitHub Action 🌎
Sign: bf293780ee827f578864d92193b8c2866acd459f
Hi @himself65, thanks for the PR...there seems to be some issues with CSR:
On initial load and route change I'm not getting past the loading skeletons:
Screenshot 2025年02月07日 at 2 23 03 PM
Hi @himself65, thanks for the PR...there seems to be some issues with CSR:
On initial load and route change I'm not getting past the loading skeletons:
Screenshot 2025年02月07日 at 2 23 03 PM
thanks for checking. I see there is a process somewhere, let me check
392b618 to
2e73145
Compare
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.
Thanks for the PR @himself65! For context, I believe the polyfill package was initially added to provide support for our postman library (see #422). Since this package has since been removed from the project, I think your solution should be good to go.
@sserrata Thoughts?
To clarify, node-polyfill-webpack-plugin was introduced as a convenient way to avoid defining specific loaders for modules no longer present in Webpack 5.
Our theme still relies on both the postman-code-generators and postman-collection libraries, although it's possible later updates to these libraries could have reduced or changed the need to define those loaders (e.g. url, process, etc.). We moved from forked to canonical versions of both libraries in #861.
@himself65 thanks for the work on replacing my use of zlib with the browser-ready pako! So far it appears to function as intended.
Uh oh!
There was an error while loading. Please reload this page.
Description
Node.js core modules are imported in client(browser) side. And the previsous code use node polyfill, however this would brings more issue:
node:buffer,node:zlib... wherenode:as prefix would still break the webpack bundler becausenode-polyfill-webpack-plugindidn't handle it correctlyBuffercould leads unexpected behaviorThis PR remove node.js dependency, instead now uses pika and web api directly to reduce the bundle hack, bundle size and more explicit imports
Motivation and Context
For more context, docusaurus will raise error if use nolyfill
telegram-cloud-photo-size-5-6138468470803777616-y
How Has This Been Tested?
Try this in demo and everything works
Screenshots (if appropriate)
Types of changes
Checklist