cap-js / cds-types

Type definitions for CDS
Apache License 2.0
10 stars 10 forks source link

feat: adds 'localized' variants to SELECT #274

Closed stockbal closed 2 weeks ago

stockbal commented 4 weeks ago

Adds all possible variants with localized to SELECT

SELECT.one.localized(Foos)
SELECT.from.localized(Foos)
SELECT.localized(Foos));
SELECT.localized.from(Foos));

Extract from @sap/cds/lib/ql/SELECT.js

...
return $((..._)       => new this()._select_or_from(..._), {
  localized: $((...x) => new this({localized:true})._select_or_from(...x),{
    columns: (..._)   => new this({localized:true}).columns(..._),
    from: (..._)      => new this({localized:true}).from(..._),
  }),
  one: $((...x)       => new this({one:true})._select_or_from(...x),{
    localized: (..._) => new this({one:true,localized:true}).from(..._)
  }),
  from: $((..._)      => new this().from(..._), {
    localized: (..._) => new this({localized:true}).from(..._)
  }),
})
...

Closes #257

daogrady commented 3 weeks ago

Hi Ludwig,

looks great! I'm just unsure about instance methods, which are not explicitly mentioned in the changelog. I.e., today the types allow us to do:

new SELECT(…).columns(…)

so I guess it should be possible to do

new SELECT(…).localized(…)

etc. as well. @mariayord do you happen to know about this or could you please forward this question to a runtime maintainer familiar with the matter? Thanks!

Best, Daniel

mariayord commented 2 weeks ago

Hi @daogrady ,

The QL API should be used only with static calls, as it is documented in https://cap.cloud.sap/docs/node.js/cds-ql#select.

Best Regards Maria

daogrady commented 2 weeks ago

Hi @mariayord ,

I see, thanks for the clarification! Guess we should also add private constructor(); to all the query classes then. But that is beyond the scope of this PR and I will take care of that[^1].

@stockbal with that we should be able to merge your contribution. Thanks for handing it in!

Best, Daniel

[^1]: see https://github.com/cap-js/cds-types/pull/290