lightbend / kalix-javascript-sdk

JavaScript and TypeScript SDKs for Kalix
https://docs.kalix.io/javascript/index.html
Apache License 2.0
22 stars 21 forks source link

chore: updating samples from file to codegen #417

Closed franciscolopezsancho closed 2 years ago

franciscolopezsancho commented 2 years ago

References https://github.com/lightbend/kalix-docs/issues/1474 and https://github.com/lightbend/kalix-javascript-sdk/pull/411

franciscolopezsancho commented 2 years ago

@johanandren from https://docs.kalix.io/javascript/proto.html

We recommend that you define your service API and events and data associated with components separately. This allows business logic to evolve independently of the public interface. This page walks you through elements in an example shoppingcart_api.proto file and the associated shoppingcart_domain.proto file.

On the other hand in the JVM samples we have the entities defined in the .api. Just to double check we don't recommend the above anymore. Would you agree?

pvlugter commented 2 years ago

from https://docs.kalix.io/javascript/proto.html

We recommend that you define your service API and events and data associated with components separately. This allows business logic to evolve independently of the public interface. This page walks you through elements in an example shoppingcart_api.proto file and the associated shoppingcart_domain.proto file.

On the other hand in the JVM samples we have the entities defined in the .api. Just to double check we don't recommend the above anymore. Would you agree?

We should still have the recommended separation for the protobuf messages, with a separate domain proto file. It's only the kalix annotations that are no longer split over the two files.

Would make sense to align with the JVM docs, which still have the above recommendation as well.

https://docs.kalix.io/java/writing-grpc-descriptors-protobuf.html

franciscolopezsancho commented 2 years ago

I missed that in the JVM docs. Thanks @pvlugter

franciscolopezsancho commented 2 years ago

I made all the changes but the error we are getting here - mongo-related - in the CI I think has nothing to do with my changes. I downloaded a fresh repo from main and it raises the same error when npm run build the ts-value-entity created with npm-js/create-kalix-entity/create-kalix-entity.js.

So far I've been unable to fix it.

johanandren commented 2 years ago

Hmm, yeah looks unrelated, what's up with something Mongo being a dependency at all?

node_modules/@types/mongodb/index.d.ts:1198:45 - error TS2344: Type 'TSchema' does not satisfy the constraint '{ [key: string]: any; }'.

franciscolopezsancho commented 2 years ago

Good point. Maybe removing it would do 👍. I'll have a look to see if doesn't break something else.

franciscolopezsancho commented 2 years ago

TODO: try adding

  compilerOptions: {
    "skipLibCheck": true,
    //...
  }

to tsconfig