Repository navigation
bump webassemblyjs - #7732
bump webassemblyjs#7732
Conversation
|
For maintainers only:
|
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
|
Please do not merge this yet. |
|
Ping me when this is ready to merge |
| const WebAssemblyUtils = require("./WebAssemblyUtils"); | ||
| const { RawSource } = require("webpack-sources"); | ||
|
|
||
| const { shrinkPaddedLEB128 } = require("@webassemblyjs/wasm-opt"); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Alright @sokra, lgtm |
| "@webassemblyjs/wasm-gen" "1.5.13" | ||
| "@webassemblyjs/wasm-parser" "1.5.13" | ||
| debug "^3.1.0" | ||
| "@babel/cli" "^7.0.0-beta.54" |
| version "1.7.4" | ||
| resolved "https://registry.yarnpkg.com/@webassemblyjs/ieee754/-/ieee754-1.7.4.tgz#b58c92c364a954a027d7415adc1bf10dd74d7ac5" | ||
| dependencies: | ||
| "@xtuc/buffer" "^5.2.2" |
There was a problem hiding this comment.
node.js has a buildin Buffer.
This seem to be for browser build only. Bundlers usually bring their own polyfills for Buffer. Polyfilling is not the responsibility of a library. If it's only needed to local building of a browser bundle, make it a dev dependency.
There was a problem hiding this comment.
Yes, that's right. Let me remove it.
| dependencies: | ||
| "@babel/highlight" "7.0.0-beta.54" | ||
|
|
||
| "@babel/core@^7.0.0-beta.54": |
| "@webassemblyjs/ast" "1.5.13" | ||
| "@webassemblyjs/wast-parser" "1.5.13" | ||
| long "^3.2.0" | ||
| "@xtuc/[email protected]": |
There was a problem hiding this comment.
Weird that you are forking packages just for ESM. This seems unnecessary to me. Why not using the original package?
There was a problem hiding this comment.
Yeah the issue is that some bundler (Rollup) won't bundle any cjs require, so I had to ESM'ify some dependecies.
For now I preferred forking them to avoid loosing my time merging it upstream, but that's definitely the plan. Is that a blocker for this PR?
There was a problem hiding this comment.
@sokra I submited PRs the upstream repositories. Would you mind removing them in a following PR? so we can fix the current issues in Webpack. If that's ok to you i'll update the PR to be ready to be merged.
webpack-bot
added
PR: reviewed-changes-requested
and removed
PR: review-outdated
labels
|
Please merge this soon after @xtuc considers it final; the current implementation actively corrupts binaries in certain circumstances. (wasm-bindgen/wasm-bindgen#753) |
sokra
left a comment
sokra
left a comment
There was a problem hiding this comment.
@xtuc/long should eventually merged into upstream, but we can use it for now.
|
Please let me update webassemblyjs before merging this. |
|
I updated it to 1.7.5 |
|
Do you want to cut a new release? |
|
Ah cool thanks, yeah I just released 1.7.6 (mostly changes to the interpreter, which is not used here). |
|
Thanks |
What kind of change does this PR introduce?
Did you add tests for your changes?
Does this PR introduce a breaking change?
What needs to be documented once your changes are merged?
ref xtuc/webassemblyjs#407