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

Emit WASM #3897

Merged
merged 13 commits into from
Nov 3, 2024
Merged

Emit WASM #3897

merged 13 commits into from
Nov 3, 2024

Conversation

Quafadas
Copy link
Contributor

@Quafadas Quafadas commented Nov 2, 2024

Seeks to address #3837

I have some outstanding questions;

  1. As I'm altering the scalaJSModule public API, a double check here would be welcomed. In particular, I've just called the module property emitWasm. sjrd pretty clearly called the linker API withExperimentalUseWebAssembly. However, if at some point it became non-experimental, I assume that's a compatibility problem. Advice welcomed - I've gone with the simplest name possible - emitWasm.
  2. CI will fail, because node is < 22. The runnable test does work locally for me. Does mill require, assume or otherwise advertise compatibility with any particular node version? If not, I'll simply push the node version forward in the GHA definitions. OItherwise, I'd welcome advice - I'm not clear what the best way to manage the CI would be.

Those are the two points where I'd welcome advice. I think this is the path to victory;

  • public API check?
  • node version
  • documentation
  • CI green

@lihaoyi
Copy link
Member

lihaoyi commented Nov 3, 2024

Thanks @Quafadas!

  1. Updating the ScalaJSModule public API is fine. Let's prefix the name scalaJSEmitWasm, since it's a Scalajs specific task.
  2. Mill does not advertise any required node version, so you can require whatever version you want, just make sure to document it (e.g. in def scalaJSEmitWasm)
  3. Let's also include an example/scalalib/web/ example test with a short illustrative example and some english prose explaining what is going on

@Quafadas
Copy link
Contributor Author

Quafadas commented Nov 3, 2024

The documentation has been written somewhat blindly - I can't seem to get past this locally.

simon@Simons-Mac-mini mill-1 % mill docs.localPages
[0753/1698] docs.oldDocSources
[0753] Cloning into '/Users/simon/Code/mill-1/out/docs/oldDocSources.dest/0.11.12'...
[0753] done.
[0753] error: pathspec '0.11.12' did not match any file(s) known to git
[1698/1698] ============================================================================================================================================================================================== docs.localPages ================================================================================================================================================================================================== 1s
1 tasks failed
docs.oldDocSources os.SubprocessException: Result of git…: 1

    os.proc.call(ProcessOps.scala:230)
    build_.docs.package_.$anonfun$oldDocSources$4(package.mill:247)
    scala.collection.immutable.List.map(List.scala:247)
    scala.collection.immutable.List.map(List.scala:79)
    build_.docs.package_.$anonfun$oldDocSources$3(package.mill:244)

@lefou
Copy link
Member

lefou commented Nov 3, 2024

@Quafadas Could be you have a sparse git checkout. You can use mill docs.fastPages which should skip the older versions.

@Quafadas
Copy link
Contributor Author

Quafadas commented Nov 3, 2024

@lefou Thankyou! I think the docs works as I expect...

@Quafadas
Copy link
Contributor Author

Quafadas commented Nov 3, 2024

Pending further feedback and making CI happy, I believe this now to be mergeable.

@Quafadas Quafadas changed the title WIP: Emit wasm Emit WASM Nov 3, 2024
@lihaoyi
Copy link
Member

lihaoyi commented Nov 3, 2024

Looks good to me, thanks @Quafadas! Email me your international bank transfer details and I will close out the bounty

@lihaoyi lihaoyi merged commit 692c0dd into com-lihaoyi:main Nov 3, 2024
24 checks passed
@lefou lefou added this to the 0.12.2 milestone Nov 3, 2024
@Quafadas
Copy link
Contributor Author

Quafadas commented Nov 3, 2024

@lihaoyi r.e. bounty I'd say don't worry about it I get plenty out of mill so let's keep that resource inside the project.

You can either add it to the scala 3 migration slush fund, or if you ever get the chance to have dinner with Tobias and/or Lorenzo - make it a nice one.

@lihaoyi
Copy link
Member

lihaoyi commented Nov 3, 2024

@Quafadas got it! In that case I'll pass the bounty to the scala center then to support their work

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.

4 participants