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

chore(wasm): update NPM build script #3429

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,7 @@ web-sys = { version = "0.3.64", features = ["console"] }

[dev-dependencies]
wasm-bindgen-test = "0.3.37"
serde_json = "1.0.107"
serde_json = "1.0.107"

[profile.release]
lto = true
TalDerei marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion crates/wasm/publish/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use Rust functionality via .wasm.
# Install wasm-pack first: https://rustwasm.github.io/wasm-pack/installer/

npm install
npm run publish-wasm
npm run publish-wasm-bundle
```

We have a release github action that runs these on every release.
9 changes: 5 additions & 4 deletions crates/wasm/publish/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@
"scripts": {
"format": "prettier --write .",
"compile-wasm": "tsc && node build/run.js",
"publish-wasm": "npm run compile-wasm -- --publish"
"publish-wasm": "npm run compile-wasm -- --publish",
"publish-wasm-bundle": "npm run compile-wasm -- --publish --bundle"
},
"dependencies": {
"wasm-pack": "^0.12.1"
},
"devDependencies": {
"@types/node": "^20.5.6",
"@types/node": "^20.9.4",
"prettier": "^3.0.2",
"tsx": "^3.12.7",
"typescript": "^5.2.2",
"prettier": "^3.0.2"
"typescript": "^5.2.2"
}
}
87 changes: 82 additions & 5 deletions crates/wasm/publish/run.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is out of scope, but I wonder if we should re-write this little script in Rust. Initially, the thinking was that this was low weight and the sync methods weren't that big of a deal. Now that we likely need to do this six times (3 targets x 2 bin bundle options), I think we should introduce parallelism here. And if so, perhaps Rust (multithreading w/ Rayon) makes this easier and fits more within the wider repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we keep it in Typescript, think we should do something like:

import util from 'util';
import { exec } from 'child_process';

const asyncExec = util.promisify(exec);

const TARGETS = ['bundler', 'web', 'nodejs'];

(async function () {
  const promises = TARGETS.map(target =>
    asyncExec(
      `wasm-pack build ../ --release --target ${target} --out-name index --out-dir publish/${target}-${binBundle}`,
    ),
  );
  await Promise.all(promises);
})();

or perhaps this library is easier to use and more visually discernable (the above does not have stdout): https://www.npmjs.com/package/concurrently

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think rewriting the script in Rust might be a bit out of scope for now. I can experiment with these suggestions!

Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import path from 'path';
import { execSync } from 'child_process';
import { readFileSync, writeFileSync } from 'fs';
import { copyFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs';

const TARGETS = ['web', 'nodejs', 'bundler'];
const TARGETS_WITHOUT_BIN = ['web', 'nodejs', 'bundler'];
const TARGETS_WITH_BIN = ['web-bin', 'nodejs-bin', 'bundler-bin'];

TARGETS.forEach(target => {
TARGETS_WITHOUT_BIN.forEach(wasmCompile);
TARGETS_WITH_BIN.forEach(wasmCompileBinary);

function wasmCompile(target: string): void {
// Run wasm-pack for each target
execSync(
`wasm-pack build ../ --release --target ${target} --out-name index --out-dir publish/${target}`,
Expand All @@ -16,7 +20,8 @@ TARGETS.forEach(target => {
// Rename package to target-specific names
const packageJsonPath = path.join(process.cwd(), `${target}/package.json`);
const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8'));
packageJson.name = `@penumbra-zone/wasm-${target}`;

packageJson.name = `@penumbra-zone-test/wasm-${target}`;
writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2), 'utf-8');

// Without packing first, the .wasm's will not be included
Expand All @@ -30,4 +35,76 @@ TARGETS.forEach(target => {

// Change working directory back to parent
process.chdir('..');
});
};

function wasmCompileBinary(target: string): void {
// Execute if 'bundle' flag is set
if (process.argv.includes('--bundle')) {
// Logically seperate the copy and build targets
const buildTarget = target.replace('-bin', '');
const copyTarget = target;

// Run wasm-pack for each target
execSync(
`wasm-pack build ../ --release --target ${buildTarget} --out-name index --out-dir publish/${copyTarget}`,
{
stdio: 'inherit',
},
);
// Copy binary files to the package directory
const binaryDir = path.join(process.cwd(), '../../crypto/proof-params/src/gen/');
const targetPackageDir = path.join(process.cwd(), `${copyTarget}`);

// Ensure the target directory exists
if (existsSync(binaryDir)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If it doesn't we should throw an error

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added error handling

const targetBinaryDir = path.join(targetPackageDir, 'bin');
if (!existsSync(targetBinaryDir)) {
mkdirSync(targetBinaryDir);
}

// Copy binary files to the package directory
const binaryFiles = [
'delegator_vote_pk.bin',
'nullifier_derivation_pk.bin',
'output_pk.bin',
'spend_pk.bin',
'swap_pk.bin',
'swapclaim_pk.bin',
'undelegateclaim_pk.bin'
];
binaryFiles.forEach(file => {
const sourcePath = path.join(binaryDir, file);
const targetPath = path.join(targetBinaryDir, file);
copyFileSync(sourcePath, targetPath);
});
} else {
// Throw error if target directory doesn't exist
throw new Error(`The directory ${binaryDir} does not exist.`);
}

// Rename package to target-specific names
const packageJsonPath = path.join(process.cwd(), `${copyTarget}/package.json`);
const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8'));

// Check if the 'files' property in the generated package.json includes the
// bin directory, otherwise include it. Without this line, wasm-pack will
// fail to bundle the binary proving keys inside the NPM package.
if (!packageJson.files.includes('bin')) {
packageJson.files.push('bin');
}
packageJson.name = `@penumbra-zone-test/wasm-${copyTarget}`;
writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2), 'utf-8');

// Without packing first, the .wasm's will not be included
process.chdir(copyTarget);
execSync('npm pack', { stdio: 'inherit' });

// Publish to npm if flag provided
if (process.argv.includes('--publish')) {
execSync('npm publish --access public', { stdio: 'inherit' });
}

// Change working directory back to parent
process.chdir('..');
}
};