Skip to content

Client wasm multiple targets #2135

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

Merged
merged 9 commits into from
Nov 25, 2024
Merged

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Nov 22, 2024

Content

This PR rework our npm published mithril-client-wasm package in order to include multiple targets (node, web, bundler) instead of only the web target.
This allow Node.Js users to use our published package instead of needing to manually build a package.

This works by defining manually a package.json to aggregates built wasm-pack targets into a single package.
Inside it we set the main and the browser entrypoints so child projects can automatically choose the relevant targets between node and web meaning that existing client should not needs any change.

Note

Some clients that use mixed projects types (typically that use SSR, ie: NextJs) can have trouble with the automatic detection, in this case they can target a specific package in their package.json

Example to target directly the web package:

 "dependencies": {
   "@mithril-dev/mithril-client-wasm": "file:../mithril-client-wasm/dist/web",
 },

todo: update wasm readme (compat table and fallbacks).

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)

Issue(s)

Closes #2091

@Alenar Alenar self-assigned this Nov 22, 2024
Copy link

github-actions bot commented Nov 22, 2024

Test Results

    4 files  ±0     51 suites  ±0   11m 42s ⏱️ -9s
1 455 tests ±0  1 455 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 666 runs  ±0  1 666 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 9e8e19e. ± Comparison against base commit dfb1eea.

♻️ This comment has been updated with latest results.

@Alenar Alenar force-pushed the ensemble/2091/client-wasm-multiple-targets branch from 673e990 to fe8b486 Compare November 22, 2024 10:13
@Alenar Alenar temporarily deployed to testing-preview November 22, 2024 10:24 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet November 22, 2024 10:25 — with GitHub Actions Inactive
@Alenar Alenar force-pushed the ensemble/2091/client-wasm-multiple-targets branch from eb13b9e to 1dd0dfb Compare November 22, 2024 13:57
@Alenar Alenar marked this pull request as ready for review November 22, 2024 15:22
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Alenar Alenar temporarily deployed to testing-preview November 22, 2024 15:24 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet November 22, 2024 15:24 — with GitHub Actions Inactive
@Alenar Alenar force-pushed the ensemble/2091/client-wasm-multiple-targets branch from 1dd0dfb to 2e46884 Compare November 22, 2024 16:19
Copy link
Collaborator

@dlachaume dlachaume left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Alenar Alenar force-pushed the ensemble/2091/client-wasm-multiple-targets branch from 2e46884 to 347430d Compare November 22, 2024 16:32
Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM

Alenar and others added 4 commits November 25, 2024 11:16
+ adapt www, www-test, and explorer `package.json` and upgrade their
  dependencies

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
- Move nodejs & web browser examples to the `/examples` directory
  - Standardize them: add a README and a MAKEFILE when missing
- Rename `mithril-client-wasm/www-test` to `ci-test`
- adapt ci
- adapt upgrade repository dependencies devbook

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
@Alenar Alenar temporarily deployed to testing-preview November 25, 2024 10:17 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet November 25, 2024 10:17 — with GitHub Actions Inactive
@Alenar Alenar force-pushed the ensemble/2091/client-wasm-multiple-targets branch from 347430d to b0936ca Compare November 25, 2024 10:33
Alenar and others added 2 commits November 25, 2024 11:34
by targetting explicitly the web package, else NextJs want to target
both the node package for it's Server side rendering and the web package
for the client side. It then fail to find the node package and fallback
to client rendering with an error.

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
@Alenar Alenar force-pushed the ensemble/2091/client-wasm-multiple-targets branch from b0936ca to 773eb19 Compare November 25, 2024 10:35
@Alenar Alenar temporarily deployed to testing-preview November 25, 2024 10:50 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet November 25, 2024 10:50 — with GitHub Actions Inactive
As previous version `8.57.1` is now deprecated
Alenar and others added 2 commits November 25, 2024 11:56
* mithril-client-wasm from `0.6.1` to `0.7.0`
* [js] mithril-client-wasm from `0.6.1` to `0.6.2`
* [js] mithril-explorer from `0.7.18` to `0.7.19`

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>
@Alenar Alenar force-pushed the ensemble/2091/client-wasm-multiple-targets branch from 773eb19 to 9e8e19e Compare November 25, 2024 10:56
@Alenar Alenar temporarily deployed to testing-preview November 25, 2024 11:05 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet November 25, 2024 11:05 — with GitHub Actions Inactive
@Alenar Alenar merged commit 901f130 into main Nov 25, 2024
47 checks passed
@Alenar Alenar deleted the ensemble/2091/client-wasm-multiple-targets branch November 25, 2024 11:29
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.

Make client WASM npm package compatible with NodeJS
4 participants