DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

allow for loading wasm without unsafe wasm execution #1793

Open
cramt wants to merge 1 commits from cramt/wasm_unsafe_eval_fix into main
pull from: cramt/wasm_unsafe_eval_fix
merge into: DaddyFrosty:main
Conversation 3 Commits 1 Files Changed 4 +66 -13
cramt commented 2025-03-02 11:33:53 -08:00 (Migrated from github.com)
Copy Link

fixes #1767, by allowed you to not bundle the wasm as a base64 encoded string which is dynamically loaded at eval time

fixes #1767, by allowed you to not bundle the wasm as a base64 encoded string which is dynamically loaded at eval time
🎉 2 🚀 1
vercel[bot] commented 2025-03-02 11:33:56 -08:00 (Migrated from github.com)
Copy Link

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yoga-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 2, 2025 7:36pm
[vc]: #viaJP9DcMumGk4p1f3GZFuMWST2Qc31Rks0DSv1GCzg=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ5b2dhLXdlYnNpdGUiLCJyb290RGlyZWN0b3J5Ijoid2Vic2l0ZSIsImluc3BlY3RvclVybCI6Imh0dHBzOi8vdmVyY2VsLmNvbS9mYm9wZW5zb3VyY2UveW9nYS13ZWJzaXRlLzc3MlJyM25BVnJXSncydlZKWTNrc05TQlpVM0EiLCJwcmV2aWV3VXJsIjoieW9nYS13ZWJzaXRlLWdpdC1mb3JrLWNyYW10LXdhc211bnNhZmVldmFsZml4LWZib3BlbnNvdXJjZS52ZXJjZWwuYXBwIiwibmV4dENvbW1pdFN0YXR1cyI6IkRFUExPWUVEIiwibGl2ZUZlZWRiYWNrIjp7InJlc29sdmVkIjowLCJ1bnJlc29sdmVkIjowLCJ0b3RhbCI6MCwibGluayI6InlvZ2Etd2Vic2l0ZS1naXQtZm9yay1jcmFtdC13YXNtdW5zYWZlZXZhbGZpeC1mYm9wZW5zb3VyY2UudmVyY2VsLmFwcCJ9fV19 **The latest updates on your projects**. Learn more about [Vercel for Git ↗︎](https://vercel.link/github-learn-more) | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **yoga-website** | ✅ Ready ([Inspect](https://vercel.com/fbopensource/yoga-website/772Rr3nAVrWJw2vVJY3ksNSBZU3A)) | [Visit Preview](https://yoga-website-git-fork-cramt-wasmunsafeevalfix-fbopensource.vercel.app) | 💬 [**Add feedback**](https://vercel.live/open-feedback/yoga-website-git-fork-cramt-wasmunsafeevalfix-fbopensource.vercel.app?via=pr-comment-feedback-link) | Mar 2, 2025 7:36pm |
NickGerleman (Migrated from github.com) reviewed 2025-03-05 13:45:36 -08:00
javascript/CMakeLists.txt
NickGerleman (Migrated from github.com) commented 2025-03-05 13:45:33 -08:00
Copy Link

Could we avoid duplicating the bits here that are not different?

Could we avoid duplicating the bits here that are not different?
javascript/CMakeLists.txt
NickGerleman (Migrated from github.com) commented 2025-03-05 13:45:01 -08:00
Copy Link

From what I learned in the other threads, DYNAMIC_EXECUTION=0 might be making the bindings quite a bit slower, if we still end up using it anyways.

Might be worth removing this argument from the base64 variant, or seeing what happens if we add EMBIND_AOT to both (might require bumping emsdk here 1b7d2c8d48/javascript/just.config.cjs (L140))

From what I learned in the other threads, `DYNAMIC_EXECUTION=0` might be making the bindings quite a bit slower, if we still end up using it anyways. Might be worth removing this argument from the base64 variant, or seeing what happens if we add `EMBIND_AOT` to both (might require bumping emsdk here https://github.com/facebook/yoga/blob/1b7d2c8d48e83fb136eccc5215faeb8d54da9c3e/javascript/just.config.cjs#L140)
NickGerleman commented 2025-03-05 13:45:42 -08:00 (Migrated from github.com)
Copy Link

Thanks for digging into this issue!

I've been a little bit weary towards wanting many flavors. E.g. some folks have wanted CommonJS, others have wanted an ASM.js version, not to mention the sync compile vs async compile delineation we have right now. But it seems like this issue is pretty commonly hit, so this makes sense to me.

When we did have multiple flavors though, for sanity, we ran Jest tests against each of them. That would be great to add back here (some of it was in the before state for) ef1d772447

I haven't thought about it closely, but I almost wonder if it might make sense to have different packages for these.

Thanks for digging into this issue! I've been a little bit weary towards wanting many flavors. E.g. some folks have wanted CommonJS, others have wanted an ASM.js version, not to mention the sync compile vs async compile delineation we have right now. But it seems like this issue is pretty commonly hit, so this makes sense to me. When we did have multiple flavors though, for sanity, we ran Jest tests against each of them. That would be great to add back here (some of it was in the before state for) https://github.com/facebook/yoga/commit/ef1d772447c53266f57263e96420e24536b38d5d I haven't thought about it closely, but I almost wonder if it might make sense to have different packages for these.
👍 1
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin cramt/wasm_unsafe_eval_fix:cramt/wasm_unsafe_eval_fix
git checkout cramt/wasm_unsafe_eval_fix
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
Algorithm
Build and Infrastructure
C#
C++ Enhancement
CLA Signed
Discussion
Documentation
Feature Request
Java
JavaScript
Merged
Question
Reverted
Shared with Meta
Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
YogaKit
YogaLayout ViewGroup
bug
dependencies
Pull requests that update a dependency file
duplicate
enhancement
fb-exported
fh:direct-merge-enabled
invalid
ruby
Pull requests that update Ruby code
wontfix
No Label
CLA Signed
Shared with Meta
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
DaddyFrosty
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#1793
No description provided.
Delete Branch "cramt/wasm_unsafe_eval_fix"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?