Skip to content

Wasm build and publish - #3316

Open
reynoldsnlp wants to merge 10 commits into
praat:masterfrom
reynoldsnlp:wasm
Open

Wasm build and publish#3316
reynoldsnlp wants to merge 10 commits into
praat:masterfrom
reynoldsnlp:wasm

Conversation

@reynoldsnlp

Copy link
Copy Markdown

This wasm build is already passing tests and I published to npm. A deployed example is here.

contents: read
packages: write
steps:
- uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this should probably use the latest version of the action (i.e. v6 as of now). Same for all other official actions

Comment thread wasm/js/praat-wasm.d.ts
removeSelected(): void;

/** Direct access to Emscripten's virtual filesystem. */
FS: any;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this might use the types from @types/emscripten to define the proper type here instead of using any

Comment thread wasm/package.json
"types": "./js/praat-wasm.d.ts"
},
"./classes": {
"import": "./js/classes.mjs"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If this file is exposed for direct import, it should probably have its type declarations to make it usable in Typescript (the praat-wasm.d.ts file can then reference classes.d.ts instead of duplicating them)

Comment thread wasm/package.json
},
"./worker-client": {
"import": "./js/worker-client.mjs",
"types": "./js/praat-wasm.d.ts"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this looks wrong to me. worker-client.mjs does not export the full praat-wasm API. It exports only createPraatWorker. This should have a worker-client.d.ts file matching the API of that file

Comment thread wasm/package.json
],
"repository": {
"type": "git",
"url": "git+https://github.com/reynoldsnlp/praat.github.io.git"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should probably be updated to be the praat repository

Comment thread wasm/package.json
"import": "./js/classes.mjs"
},
"./worker": {
"import": "./js/worker.mjs"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this one should also have type declarations

Comment thread wasm/dist/praat.wasm

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should the generated wasm file really be committed in git ?

Comment thread wasm/js/classes.mjs
* Add snake_case aliases for all camelCase methods on a prototype.
* @param {Function} cls
*/
function addSnakeCaseAliases (cls) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need those snake case aliases ? That's quite uncommon in JS packages

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.

2 participants