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

web: Don't unnecessarily duplicate the module for single WASM builds #19198

Merged

Conversation

danielhjacobs
Copy link
Contributor

No description provided.

@danielhjacobs danielhjacobs added A-web Area: Web & Extensions T-refactor Type: Refactor / Cleanup labels Jan 13, 2025
@danielhjacobs danielhjacobs force-pushed the dont-build-standin-for-wasm branch 2 times, most recently from b571a37 to 7412f57 Compare January 14, 2025 14:45
Copy link
Member

@torokati44 torokati44 left a comment

Choose a reason for hiding this comment

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

The diff looks okay, though I suppose this can be squashed into a single commit. Thank you!

@Dinnerbone
Copy link
Contributor

We can remove the copying code from build_wasm.ts now, right?

@torokati44
Copy link
Member

Hah, good point, this is only the loading part so far... 😅

@danielhjacobs danielhjacobs force-pushed the dont-build-standin-for-wasm branch from 012e13c to ce91845 Compare January 14, 2025 20:43
@danielhjacobs
Copy link
Contributor Author

Ah, the copying part complains about these:

import type { RuffleInstanceBuilder, ZipWriter } from "../dist/ruffle_web";

import type { RuffleInstanceBuilder } from "../../dist/ruffle_web";

import type { RuffleHandle, ZipWriter } from "../../../dist/ruffle_web";

@torokati44
Copy link
Member

You mean, if it's not copied, then the build complains about it missing?

@danielhjacobs
Copy link
Contributor Author

danielhjacobs commented Jan 14, 2025

Yes, but I can @ts-expect-error and switch the type imports to come from %FALLBACK_WASM%.

@Dinnerbone
Copy link
Contributor

Dinnerbone commented Jan 14, 2025

Those should be pointing to the one that always exists, which is now -wasm_extensions, right?

(we probably should have kept just the name ruffle_web as the one that always exists and renamed the other one... but oh well)

@danielhjacobs danielhjacobs force-pushed the dont-build-standin-for-wasm branch from ce91845 to c7dd2e9 Compare January 14, 2025 21:06
@danielhjacobs
Copy link
Contributor Author

Fixed

@danielhjacobs danielhjacobs added the waiting-on-review Waiting on review from a Ruffle team member label Jan 14, 2025
@danielhjacobs danielhjacobs force-pushed the dont-build-standin-for-wasm branch from c7dd2e9 to 719b94e Compare January 14, 2025 21:59
@torokati44 torokati44 removed the waiting-on-review Waiting on review from a Ruffle team member label Jan 14, 2025
@torokati44 torokati44 merged commit becce39 into ruffle-rs:master Jan 14, 2025
20 checks passed
@torokati44
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web Area: Web & Extensions T-refactor Type: Refactor / Cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants