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.

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

Jump to bottom

Separate types definition from the sdk so web app client side could import them #50

Merged
merged 13 commits into from
Jan 7, 2025
Merged

Separate types definition from the sdk so web app client side could import them #50

merged 13 commits into from
Jan 7, 2025

Conversation

chrisli30
Copy link
Member

@chrisli30 chrisli30 commented Jan 4, 2025 •
edited
Loading

Fix #49. This solves the http2 issue that blocked import on the next.js client side. The http2 is required by @grpc/grpc-js depenedency which we can’t really get rid of, so my idea is to create a separate light-weight library for the types import.

These changes have been published into the following libraries, and tested in web app.
@avaprotocol/sdk-js@1.3.0
@avaprotocol/types@0.9.2-dev.0

Note that
Since we need yarn workspace for the monorepo, I switched from npm to yarn for managing depenedencies.

There are a few changes before this PR that were accidentally checked into main:

  1. Removed dist folder from git
  2. Wrapped WorkflowStatus from number value to string representation

@socket-security Socket Security
Copy link

socket-security bot commented Jan 4, 2025 •
edited
Loading

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@avaprotocol/types@0.9.2-dev.4 None 0 0 B
npm/@babel/code-frame@7.26.2 environment 0 33.2 kB nicolo-ribaudo
npm/@babel/compat-data@7.26.3 None 0 66.5 kB nicolo-ribaudo
npm/@babel/generator@7.26.3 None 0 546 kB nicolo-ribaudo
npm/@babel/parser@7.26.3 None 0 1.9 MB nicolo-ribaudo
npm/@babel/plugin-transform-exponentiation-operator@7.26.3 None 0 11.5 kB nicolo-ribaudo
npm/@babel/plugin-transform-modules-commonjs@7.26.3 None 0 41.7 kB nicolo-ribaudo
npm/@babel/plugin-transform-typescript@7.26.3 None 0 137 kB nicolo-ribaudo
npm/@babel/traverse@7.26.4 Transitive: environment +1 716 kB nicolo-ribaudo
npm/@changesets/apply-release-plan@7.0.7 filesystem 0 187 kB changesets-release-bot
npm/@changesets/assemble-release-plan@6.0.5 None 0 159 kB changesets-release-bot
npm/@changesets/cli@2.27.11 environment, filesystem, shell +1 224 kB changesets-release-bot
npm/@changesets/config@3.0.5 filesystem 0 41.8 kB changesets-release-bot
npm/@changesets/get-release-plan@4.0.6 None 0 27 kB changesets-release-bot
npm/@changesets/git@3.0.2 filesystem 0 71.8 kB changesets-release-bot
npm/@changesets/read@0.6.2 filesystem 0 34.2 kB changesets-release-bot
npm/@esbuild/aix-ppc64@0.24.2 None 0 0 B
npm/@esbuild/android-arm@0.24.2 None 0 11.9 MB esbuild, evanw
npm/@esbuild/android-arm64@0.24.2 None 0 10.6 MB esbuild, evanw
npm/@esbuild/android-x64@0.24.2 None 0 11.9 MB esbuild, evanw
npm/@esbuild/darwin-arm64@0.24.2 None 0 9.75 MB esbuild, evanw
npm/@esbuild/darwin-x64@0.24.2 None 0 10.4 MB esbuild, evanw
npm/@esbuild/freebsd-arm64@0.24.2 None 0 9.5 MB esbuild, evanw
npm/@esbuild/freebsd-x64@0.24.2 None 0 10.2 MB esbuild, evanw
npm/@esbuild/linux-arm@0.24.2 None 0 9.63 MB esbuild, evanw
npm/@esbuild/linux-arm64@0.24.2 None 0 9.57 MB esbuild, evanw
npm/@esbuild/linux-ia32@0.24.2 None 0 9.6 MB esbuild, evanw
npm/@esbuild/linux-loong64@0.24.2 None 0 9.77 MB esbuild, evanw
npm/@esbuild/linux-mips64el@0.24.2 None 0 11 MB esbuild, evanw
npm/@esbuild/linux-ppc64@0.24.2 None 0 0 B
npm/@esbuild/linux-riscv64@0.24.2 None 0 9.44 MB esbuild, evanw
npm/@esbuild/linux-s390x@0.24.2 None 0 10.7 MB esbuild, evanw
npm/@esbuild/linux-x64@0.24.2 None 0 10.2 MB esbuild, evanw
npm/@esbuild/netbsd-arm64@0.24.2 None 0 10.1 MB esbuild, evanw
npm/@esbuild/netbsd-x64@0.24.2 None 0 10.1 MB esbuild, evanw
npm/@esbuild/openbsd-arm64@0.24.2 None 0 9.57 MB esbuild, evanw
npm/@esbuild/openbsd-x64@0.24.2 None 0 10.2 MB esbuild, evanw
npm/@esbuild/sunos-x64@0.24.2 None 0 0 B
npm/@esbuild/win32-arm64@0.24.2 None 0 9.58 MB esbuild, evanw
npm/@esbuild/win32-ia32@0.24.2 None 0 9.86 MB esbuild, evanw
npm/@esbuild/win32-x64@0.24.2 None 0 10.4 MB esbuild, evanw
npm/@eslint/js@9.17.0 None 0 14.9 kB eslintbot, openjsfoundation
npm/@grpc/grpc-js@1.12.5 environment, filesystem, network +1 2.41 MB murgatroid99
npm/@jridgewell/gen-mapping@0.3.8 None 0 81.6 kB jridgewell
npm/@rollup/rollup-android-arm-eabi@4.29.2 None 0 1.61 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-android-arm64@4.29.2 None 0 2.23 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-darwin-arm64@4.29.2 None 0 2.19 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-darwin-x64@4.29.2 None 0 2.37 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-freebsd-arm64@4.29.2 None 0 2.12 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-freebsd-x64@4.29.2 None 0 2.45 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-arm-gnueabihf@4.29.2 None 0 2.25 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-arm-musleabihf@4.29.2 None 0 2.24 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-arm64-gnu@4.29.2 None 0 2.22 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-arm64-musl@4.29.2 None 0 2.12 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-loongarch64-gnu@4.29.2 None 0 2.56 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-powerpc64le-gnu@4.29.2 None 0 2.76 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-riscv64-gnu@4.29.2 None 0 2.27 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-s390x-gnu@4.29.2 None 0 3.83 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-x64-gnu@4.29.2 None 0 2.5 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-linux-x64-musl@4.29.2 None 0 2.48 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-win32-arm64-msvc@4.29.2 None 0 2.75 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-win32-ia32-msvc@4.29.2 None 0 2.56 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@rollup/rollup-win32-x64-msvc@4.29.2 None 0 3.31 MB guybedford, lukastaegert, rich_harris, ...1 more
npm/@types/jest@29.5.14 None 0 78.8 kB types
npm/@types/lodash@4.17.14 None 0 868 kB types
npm/@typescript-eslint/eslint-plugin@8.19.0 None 0 2.79 MB bradzacher, jameshenry
npm/@typescript-eslint/parser@8.19.0 None 0 18.9 kB bradzacher, jameshenry
npm/@typescript-eslint/scope-manager@8.19.0 None 0 626 kB bradzacher, jameshenry
npm/@typescript-eslint/type-utils@8.19.0 None 0 127 kB jameshenry
npm/@typescript-eslint/types@8.19.0 None 0 174 kB bradzacher, jameshenry
npm/@typescript-eslint/typescript-estree@8.19.0 Transitive: environment +2 1.04 MB bradzacher, jameshenry
npm/@typescript-eslint/utils@8.19.0 None 0 288 kB bradzacher, jameshenry
npm/@typescript-eslint/visitor-keys@8.19.0 None 0 19.6 kB jameshenry
npm/babel-plugin-polyfill-corejs2@0.4.12 None 0 81.5 kB nicolo-ribaudo
npm/babel-plugin-polyfill-regenerator@0.6.3 None 0 9.74 kB nicolo-ribaudo
npm/browserslist@4.24.3 None 0 0 B
npm/bundle-require@5.1.0 environment, filesystem, unsafe 0 26.4 kB egoist
npm/caniuse-lite@1.0.30001690 None 0 2.15 MB caniuse-lite
npm/chokidar@4.0.3 environment, filesystem 0 149 kB paulmillr
npm/consola@3.3.3 None 0 320 kB atinux, pi0
npm/dotenv@16.4.7 environment, filesystem 0 75.8 kB motdotla
npm/electron-to-chromium@1.5.76 None 0 154 kB kilianvalkhof
npm/esbuild@0.24.2 None 0 134 kB esbuild, evanw
npm/eslint@9.17.0 environment Transitive: filesystem +6 3.42 MB eslintbot
npm/fast-glob@3.3.3 filesystem +1 107 kB mrmlnc
npm/fastq@1.18.0 None 0 43.5 kB matteo.collina
npm/is-core-module@2.16.1 None 0 33.5 kB ljharb
npm/jsesc@3.1.0 None 0 32.3 kB mathias
npm/lilconfig@3.1.3 filesystem 0 18 kB antonk52
npm/node-releases@2.0.19 None 0 37.2 kB chicoxyzzy
npm/package-manager-detector@0.2.8 environment 0 39.6 kB antfu
npm/readdirp@4.0.2 filesystem 0 32.2 kB paulmillr
npm/regexpu-core@6.2.0 None 0 136 kB google-wombot
npm/regjsparser@0.12.0 None +1 105 kB jviereck
npm/resolve.exports@2.0.3 None 0 24.1 kB lukeed
npm/resolve@1.22.10 environment, filesystem 0 146 kB ljharb
npm/rollup@4.29.2 None 0 2.61 MB eventualbuddha, lukastaegert, rich_harris, ...2 more
npm/spawndamnit@3.0.1 None 0 4.48 kB thejameskyle
npm/string-width-cjs@4.2.3 None 0 0 B
npm/strip-ansi-cjs@6.0.1 None 0 0 B
npm/tinyexec@0.3.2 environment, shell 0 46.1 kB 43081j
npm/tinyglobby@0.2.10 None +1 110 kB superchupu
npm/tsup@8.3.5 eval, filesystem +4 1.06 MB egoist
npm/typescript@5.7.2 None 0 22.7 MB typescript-bot
npm/wrap-ansi-cjs@7.0.0 None 0 0 B

🚮 Removed packages: npm/@changesets/cli@2.27.9, npm/@eslint/js@9.16.0, npm/@grpc/grpc-js@1.12.2, npm/@types/jest@29.5.13, npm/@types/lodash@4.17.12, npm/@typescript-eslint/eslint-plugin@8.17.0, npm/@typescript-eslint/parser@8.17.0, npm/dotenv@16.4.5, npm/eslint@9.16.0, npm/globals@15.13.0, npm/tsup@8.3.0, npm/typescript@5.6.3

View full report↗︎

@chrisli30 chrisli30 force-pushed the chris-convert_to_monorepo branch from 4123d38 to 76b62bd Compare January 4, 2025 22:29
@chrisli30 chrisli30 requested a review from v9n January 4, 2025 22:41
@chrisli30
Copy link
Member Author

chrisli30 commented Jan 4, 2025 •
edited
Loading

@v9n, could you help take a look the below home runner path issue in the failed tests?

Based on the config of jest.config.cjs:

 moduleNameMapper: {
    "^@/sdk-js/(.*)$": "<rootDir>/packages/sdk-js/$1",
    "^@/grpc_codegen/(.*)$": "<rootDir>/grpc_codegen/$1",
    "^@/types/(.*)$": "<rootDir>/packages/types/$1",
  },

tests running locally could resolve the path reference such as @/sdk-js/dist, but is the path in the below screenshot contains double ava-sdk-js. I didn’t find where it’s configured.

CleanShot 2025-01-04 at 14 40 31@2x

@chrisli30 chrisli30 mentioned this pull request Jan 7, 2025
Merged
v9n
v9n reviewed Jan 7, 2025
View reviewed changes
.github/workflows/dev-test-on-pr.yml
export TEST_API_KEY="$(docker compose exec aggregator /ava create-api-key --role=admin --subject=apikey)"
export ENDPOINT="localhost:2206"
yarn test
Copy link
Member

Choose a reason for hiding this comment

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

do we need to run yarn run build somewhere? or it's automatically run ? the test is still import from the dist file, not from the typescript file so we still need to ru build before test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, if we need dist files, yarn build from the root is required.

v9n
v9n approved these changes Jan 7, 2025
View reviewed changes
Copy link
Member

@v9n v9n left a comment

Choose a reason for hiding this comment

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

looks good to me. i think we do need to run build somewhere before test run to generate the dist package

@chrisli30 chrisli30 mentioned this pull request Jan 7, 2025
Closed
@chrisli30 chrisli30 merged commit 34fc2e2 into main Jan 7, 2025
2 of 4 checks passed
@chrisli30 chrisli30 deleted the chris-convert_to_monorepo branch January 7, 2025 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v9n v9n v9n approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert the repo to monorepo so we could separate /sdk and /types
2 participants
@chrisli30 @v9n