-
Notifications
You must be signed in to change notification settings - Fork 205
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
Integrate wasm-pack into the Game of Life tutorial #78
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 is almost ready I think!
@@ -87,19 +87,19 @@ wasm-opt -O3 -o output.wasm input.wasm | |||
### How small do these build configurations get our Game of Life `.wasm` binary? | |||
|
|||
With the default release build configuration (without debug symbols), our | |||
WebAssembly binary is 240,605 bytes: | |||
WebAssembly binary is 29,410 bytes: |
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.
We got it that small? Nice!
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.
I think this is mostly lld's --gc-sections removing unused panicking message fragments, since last time this was written, we didn't have that yet.
|
||
``` | ||
npm run bundle | ||
wasm-pack init |
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 creates a pkg
directory though?
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.
It makes the things below a bit unclear as to what is happening.
Are we making a pkg
directory and webpack
knows to put this in www
? Overall I'm confused as to what's going on here
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.
Because of npm link
any changes to the pkg
directory are reflected in www
's linked dependency on wasm-game-of-life
.
git clone https://github.com/rustwasm/wasm_game_of_life.git | ||
cd ./wasm_game_of_life | ||
git checkout -b chapter-zero origin/chapter-zero | ||
cargo generate --git https://github.com/rustwasm/wasm-pack-template |
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.
Did we already tell people to install cargo-generate
?
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.
Yes, the setup instructions says to install cargo-generate
.
|
||
### `wasm-game-of-life/pkg/wasm_game_of_life.d.ts` | ||
|
||
The `.d.ts` file contains [TypeScript][] type declarations for the JavaScript |
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.
No link here
|
||
use wasm_bindgen::prelude::*; | ||
``` | ||
npm link |
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.
Oh okay disregard what I said earlier about pkg
|
||
That's all it takes to publish to npm! | ||
|
||
...except other folks have also done this tutorial, and therefore the |
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.
We have a --scope=
flag for this purpose so:
wasm-pack publish --scope="mgattozzi"
and I'd recommend using that
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.
That only works if you pay npm for private packages, since we don't expose --access public
or whatever npm flags yet.
Once we do, then we can definitely update this.
rustup update | ||
rustup install nightly | ||
rustup target add wasm32-unknown-unknown --toolchain nightly | ||
cargo install cargo-generate |
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 review is out of order because of the way GitHub orders things. So yeah this answers my question
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.
Great, my replies are out of order too :)
src/game-of-life/time-profiling.md
Outdated
} | ||
``` | ||
|
||
And Voila! Refresh [http://localhost:8080](http://localhost:8080) and now we |
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.
s/V/v/g
Thanks for reviewing @mgattozzi! |
Fixes #26
Note: this shouldn't be merged until we publish a new version of
create-wasm-app
tonpm
or else thenpm init
step will fail.+cc @ashleygwilliams