-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: generic components, handlers, and contexts #333
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.
LGTM but one question around ci testing of samples with sdk in pr validation etc (but maybe we just don't do that yet here?)
"@kalix-io/kalix-javascript-sdk": "file:../../../sdk/kalix-io-kalix-javascript-sdk-0.0.0.tgz", | ||
"google-protobuf": "^3.11.4", | ||
"grpc": "^1.24.9" | ||
"@kalix-io/kalix-javascript-sdk": "1.0.0-M6" |
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.
Do we have some ci-trixery that anyway tests against current "snapshot" of the SDK?
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. Similar setup to the Java/Scala SDKs — has the latest released version in the samples, but then updated to a locally built version in CI. Builds the SDK, scripts, and codegen, and then installs them for each sample. For example:
kalix-javascript-sdk/.circleci/config.yml
Lines 226 to 232 in 50563b8
cd samples/js/js-valueentity-shopping-cart | |
# don't download, used the one we built | |
export KALIX_NPMJS_CODEGEN_BINARY="${HOME}/project/codegen/js-gen-cli/target/native-image/kalix-codegen-js" | |
npm install --save ../../../sdk/kalix-io-kalix-javascript-sdk-0.0.0.tgz ../../../npm-js/kalix-scripts/kalix-io-kalix-scripts-1.0.0.tgz | |
npm install | |
npm run build | |
npm test |
Resolves #320.
Make components (and their command handlers and contexts) generic over state and events. Type parameters have defaults, to have the same untyped behaviour as before, and to be compatible with previous code.
Update the codegen to use the typed components, and to have the right types for serialised messages (currently needs to be proto interface + reflective message, until #326).
The typed components included in the template and samples (under
kalix.d.ts
) have been removed.Update samples to make use of the new typed components and codegen. The included testkits (in the template and samples) have been updated (and should be moved into the SDK or a separate testkit dependency, #80, #168).
Will push up as separate commits, to have CI run on each set of changes, and check that samples and tests are still compatible with the SDK and codegen changes.