Skip to content
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: avoid importing net/http on wasm #449

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

paralin
Copy link
Contributor

@paralin paralin commented May 4, 2024

Using goweight: https://github.com/paralin/goweight

GOOS=js GOARCH=wasm goweight | less

Without this change:

7.9 MB runtime
6.6 MB net/http
3.1 MB net
3.0 MB crypto/tls
2.0 MB reflect
1.4 MB math/big
1.3 MB crypto/x509
935 kB os
...

With this change:

7.9 MB runtime
3.1 MB net
2.0 MB reflect
935 kB os
...

Slight modifications to avoid importing net/http reduce the binary size significantly.

nhooyr and others added 2 commits April 7, 2024 09:34
Using goweight: https://github.com/paralin/goweight

GOOS=js GOARCH=wasm goweight | less

Without this change:

  7.9 MB runtime
  6.6 MB net/http
  3.1 MB net
  3.0 MB crypto/tls
  2.0 MB reflect
  1.4 MB math/big
  1.3 MB crypto/x509
  935 kB os
  ...

With this change:

  7.9 MB runtime
  3.1 MB net
  2.0 MB reflect
  935 kB os
  ...

Slight modifications to avoid importing net/http reduce the binary size significantly.

Signed-off-by: Christian Stewart <[email protected]>
@nhooyr nhooyr changed the base branch from master to dev May 5, 2024 01:35
Copy link
Owner

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I understand the reason for this change but unfortunately I cannot merge it as it breaks the API. The API must be kept 1:1 between Go and Wasm and so Dial must return http.Response.

@paralin
Copy link
Contributor Author

paralin commented May 5, 2024

Dial is a global function and the value returned on wasm is a dummy value.

Are people using these in interfaces? is that why? Other than that it seems like anyone using Dial would already be ignoring the dummy value on wasm.

@nhooyr
Copy link
Owner

nhooyr commented May 5, 2024

People could be wrapping this library in some kind of API/helper for their own code and they might be checking resp or passing it up the chain. This patch would break that.

I don't know if it's common but it doesn't seem far fetched to me. I don't have a better suggestion unfortunately. I'll leave this open for now, let's see if anyone else has thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants