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: mac arm build #3697
fix: mac arm build #3697
Conversation
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.
This seems to change a bit more than just the brotli-size version, would it be worth just bumping brotli in our package json?
Newer |
Good point @rschristian - @gengjiawen can you re-do this PR adding |
7a08d91
to
c78428a
Compare
done. |
Has the build size increased? We (unfortunately) don't seem to run compressed size action if the source hasn't been touched. Last time I checked, updating Microbundle increased the output size by a non-negligible amount. Edit: Microbundle can now also output |
Size not changed much
on this patch
|
Rounding (or truncating) to the nearest 100b doesn't exactly give the clearest view. Probably doesn't make much sense to increase output size to solve a build-time platform compat issue. Can |
The thing that scares me here is that this seems to be generating additional files 😅 |
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.
This is great! Thank you so much for taking the time to address feedback and hanging with us 👍
@JoviDeCroock @rschristian The current iteration works fine for me and seems to generate the same output as before. I don't see any additional files being generated on my end.
@marvinhagemeister generates an extra .mjs file for all packages, if that isn't an issue I am okay with merging
|
@JoviDeCroock this is actually great that microbundle generates these by default now. This means we can drop the script in |
Want to do that in a follow-up or do we do that now? 😅 |
I'm fine with a follow up. I think we dragged this PR long enough. |
The resulting |
@rschristian oh, well in that case we should probably keep our script around. Currently it copies the |
Made a PR but elected to disable the modern output. Figured relying on the overwrite would eventually trip someone up or create issues. |
This internally bump
brotli-size
, the old version contains cpp module which not compatible on m1 mac.The
d.ts
looks auto updated with microbundler update. Not sure should i revert it or leave it be.