-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
adding browser support polyfill #5274
Conversation
Your Render PR Server URL is https://web3-js-pr-5274.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-cbcrehcgqg45pi6tqeg0. |
Pull Request Test Coverage Report for Build 2891572287
💛 - Coveralls |
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.
Usually dist contents are added for tagged releases, why we need these builds in this PR?
@jdevcs the file |
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.
@jdevcs the file
web3.min.js
contains the polyfills needed for bundlers like webpack. webpack was using our main file first,lib/index.js
and would get errors during runtime, the dist content fixes this.
These files are generated with release process so in any release these will be over written, if we include in this PR.
And if we dnt include these build files , these will be generated any ways with "browser": "dist/web3.min.js",
fix.
@jdevcs got it, removing dist files from PR |
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.
As I see, you didn't add/delete any package in package.json. Why is it a lot of changes in package-lock.json?
@avkos I ran npm run build and it updated package.lock. will remove |
Hello, I am using web3.js in my jupyter lab extension, but am running into issues during installation. I think my error is related to this update, however, when I added the changes proposed here manually, it didn't solve my issue. While building my extension, I am getting this error:
Web3js is actually used here in another node package (ocean-lib) and I have tried adding that update to this library as well. Also, I have tried using web3.js previously to just send transactions and that also resulted in the same error, but for instance ethers.js did not (but now I cannot refactor my code to ethers.js since it is nested in another package). I appreciate any suggestions. |
@smejak You added changes to your |
Hi @luu-alex . Thanks, I just sent a message there. |
this PR break throws a runtime error: removing the browser field added by this PR solves the problem.
|
|
#5053 #5031 #4659 #4767
Tested using create-react-app and angular 12 worked on my end, id reccommend testing it as well
Description
Please include a summary of the changes and be sure to follow our Contribution Guidelines.
Type of change
Checklist:
npm run dtslint
with success and extended the tests and types if necessary.npm run test:cov
and my test cases cover all the lines and branches of the added code.npm run build
with success.dist/web3.min.js
in a browser.CHANGELOG.md
file in the root folder.