Skip to content

switch skipruntime-ts to pnpm #974

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.

Sign up for GitHub

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

Draft
hubyrod wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

switch skipruntime-ts to pnpm #974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dd878da
fix Makefile
hubyrod Jul 20, 2025
bad571f
switch skipruntime-ts to pnpm
hubyrod Jul 20, 2025
c190dc5
use pnpm exec playwright test instead of npx playwright
hubyrod Jul 20, 2025
fe5100e
switch workspaces
hubyrod Jul 20, 2025
3665663
try a preinstall in package.json
hubyrod Jul 20, 2025
d6594df
fix sql/Makefile
hubyrod Jul 20, 2025
39f0a44
fix test-skipruntime-ts
hubyrod Jul 20, 2025
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
11 changes: 10 additions & 1 deletion .circleci/base.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,12 @@ jobs:
steps:
- custom-checkout:
submodules: true
- run:
name: Install pnpm
command: npm install -g pnpm
Copy link
Contributor

@jberdine jberdine Jul 21, 2025

Choose a reason for hiding this comment

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

Opinions vary but FWIW I tend to prefer using the long form of options when written out in scripts, etc. to make it easier to read for people who might not have the short forms memorized.

Copy link
Contributor Author

@hubyrod hubyrod Aug 18, 2025

Choose a reason for hiding this comment

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

Absolutely. I'll make command line options explicit.

- run:
name: Typecheck and lint typescript sources
command: SKIPRUNTIME=$(pwd)/build/skipruntime npm install && npm run build && npm run lint
command: SKIPRUNTIME=$(pwd)/build/skipruntime pnpm install && pnpm run build && pnpm run lint

compiler:
docker:
Expand Down Expand Up @@ -91,6 +94,9 @@ jobs:
steps:
- custom-checkout:
submodules: true
- run:
name: Install pnpm
command: npm install -g pnpm
- run:
name: Run wasm skdb tests
no_output_timeout: 15m
Expand Down Expand Up @@ -155,6 +161,9 @@ jobs:
steps:
- custom-checkout:
submodules: true
- run:
name: Install pnpm
command: npm install -g pnpm
- run:
name: Run wasm skip runtime tests
no_output_timeout: 10m
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ dist/
*.wasm
target
**/node_modules/
.pnpm-store/
.skgw.conf
bun.lockb
/www/docs/api/
Expand Down
38 changes: 16 additions & 22 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# various build systems

.PHONY: all
all: npm build/skdb build/init.sql
all: pnpm build/skdb build/init.sql

PRETTIER_LOG_LEVEL?=warn

Expand All @@ -19,16 +19,10 @@ export SKIPRUNTIME
# skdb wasm + js client
################################################################################

.PHONY: npm
npm: $(SKDB_WASM) build/package/skdb build/package/package.json
cd build/package && npm install
.PHONY: pnpm
pnpm:
pnpm install && pnpm build

build/package/package.json:
@echo "{" > build/package/package.json
@echo " \"dependencies\": {" >> build/package/package.json
@echo " \"skdb\": \"file:skdb\"" >> build/package/package.json
@echo " }" >> build/package/package.json
@echo "}" >> build/package/package.json
Comment on lines -22 to -31
Copy link
Contributor

@jberdine jberdine Jul 21, 2025

Choose a reason for hiding this comment

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

Something you might be hitting that isn't obvious is that here the npm stuff is referring to the support for using skdb from js using npm packages.


sql/target/wasm32-unknown-unknown/dev/skdb.wasm: sql/src/*
cd sql && skargo build --target wasm32-unknown-unknown --bin skdb
Expand Down Expand Up @@ -76,7 +70,7 @@ check:

.PHONY: check-ts
check-ts:
npm install
pnpm install
bin/check-ts.sh

.PHONY: check-sh
Expand All @@ -90,7 +84,7 @@ check-sh:
clean:
rm -Rf build
find . -name 'Skargo.toml' -print0 | sed 's|Skargo.toml|target|g' | xargs -0 rm -rf
npm run clean
pnpm clean

.PHONY: clean-all
clean-all: clean
Expand Down Expand Up @@ -125,23 +119,23 @@ check-fmt:
# regenerate api docs served by docs-run from ts sources
.PHONY: docs
docs:
npm install && npm run build
cd www && rm -rf docs/api && npm install && npx docusaurus generate-typedoc
pnpm install && pnpm build
cd www && rm -rf docs/api && pnpm install && npx docusaurus generate-typedoc
Copy link
Contributor

@jberdine jberdine Jul 21, 2025

Choose a reason for hiding this comment

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

The various npx invocations should be changed to something pnpm friendly, right? Maybe pnpm exec?


# run the docs site locally at http://localhost:3000
.PHONY: docs-run
docs-run: # depends on docs, but can't be tracked reliably
cd www && npm run start
cd www && pnpm start

# generate the docs site as static files
.PHONY: docs-build
docs-build: docs
cd www && rm -rf build && npm run build
cd www && rm -rf build && pnpm build

# run the static docs site locally
.PHONY: docs-serve
docs-serve: # depends on docs-build, but can't be tracked reliably
cd www && npm run serve
cd www && pnpm serve

# update the static docs site repo
.PHONY: docs-publish
Expand Down Expand Up @@ -229,18 +223,18 @@ build/index.html: sql/js/index.html
cp $^ $@

.PHONY: check-vite
check-vite: npm
check-vite: pnpm
rm -rf build/vitejs
cp -r sql/ts/vitejs build/vitejs
cd build/vitejs && npm install;
cd build/vitejs && pnpm install;
rm -r build/vitejs/node_modules/skdb
cp -r build/package/skdb build/vitejs/node_modules/
cd build/vitejs && npm run build
cd build/vitejs && pnpm build
cd build/vitejs && node server.js
cd build/vitejs && npm run dev
cd build/vitejs && pnpm dev

.PHONY: test-bun
test-bun: npm
test-bun: pnpm
rm -rf build/bun
cp -r sql/ts/bun build/bun
cd build/bun && bun install;
Expand Down
2 changes: 1 addition & 1 deletion bin/check-ts.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ DIRS=$( jq --raw-output ".workspaces[]" package.json )

Copy link
Contributor

@jberdine jberdine Jul 21, 2025

Choose a reason for hiding this comment

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

Note that the line above reads the workspaces field of the package.json file that isn't here anymore.

for dir in $DIRS
do
"$SCRIPT_DIR"/cd_sh "$dir" "npm run build --if-present && npm run lint --if-present"
"$SCRIPT_DIR"/cd_sh "$dir" "pnpm run build --if-present && pnpm run lint --if-present"
done
4 changes: 2 additions & 2 deletions bin/release_npm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ fi

cd "$(dirname "$2")"

npm run build --if-present
pnpm run build --if-present

npm run test --if-present
pnpm run test --if-present

if [[ "$3" =~ ^([0-9]{6})$ ]];
then
Expand Down
2 changes: 1 addition & 1 deletion bin/release_npm_skdb.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fi

cd "$SCRIPT_DIR/../" || exit

make npm
make pnpm

make test-wasm

Expand Down
Loading