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.

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

[BUG] Issue with Inline Defined Types #148

Open
1 task done
tsteckenborn opened this issue Jan 24, 2024 · 4 comments
Open
1 task done

[BUG] Issue with Inline Defined Types #148

tsteckenborn opened this issue Jan 24, 2024 · 4 comments
Labels
bug Something isn't working keepalive Will not be closed by Stale bot

Comments

@tsteckenborn
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Nature of Your Project

TypeScript

Current Behavior

Given an inline defined type, such as:

context: {
title: String;
content: String;
};

Using it within QL based on it's types, it's typed as:

context?: QLExtensions_<{title?: string | null; content?: string | null }>  | undefined

Leading to implied usage as:

[...]?.context?.content

Yet that leads to a tried access on a column named CONTENT.

Correct usage here would be:

[...]?.context_content

This works, but yields a type error.

Expected Behavior

Return a correct type for usage within QL also for inline defined types.

Steps To Reproduce

No response

Environment

- **OS**: Mac OS
- **Node**: v20.10.0
- **npm**: 10.2.3
- **cds-typer**: 0.12.0
- **cds**: 7.4.1

Repository Containing a Minimal Reproducible Example

No response

Anything else?

No response

@tsteckenborn tsteckenborn added bug Something isn't working new labels Jan 24, 2024
@daogrady daogrady added the keepalive Will not be closed by Stale bot label Jan 29, 2024
@daogrady daogrady removed the new label Mar 4, 2024
@stockbal
Copy link
Contributor

Hi @tsteckenborn,

did you pass the option --inlineDeclarations flat to the CLI? The default value for inline declarations is structured.

Structured produces a type like:

...
    context?: {
    title?: string | null,
    content?: string | null,
  } | null;
...

whereas flat produces:

...
    context_title?: string | null
    context_content?: string | null
...

P.S. You can also set this option in the VS Code settings

"cds.typeGenerator.command": "node \"${typerBinary}\" \"${targetFile}\" --outputDirectory \"${outputDirectory}\" --inlineDeclarations flat"

@daogrady what is the reasoning behind structured being the default? I don't see an option in CAP to store such an inline property as a single JSON object, like it is done with array of.

@daogrady
Copy link
Contributor

Hi Ludwig,

thanks for weighing in on this matter!

what is the reasoning behind structured being the default?

That is a good question. I can not find any notes on the reasoning behind this. Looking at it today, I also believe that flat should be the default. I am a bit reluctant to change it now, as that would be a large breaking change and those are a bit harder to convey in the 0.y.z setup we are currently operating in.

Best,
Daniel

@stockbal
Copy link
Contributor

Hi Daniel,

I thought at version 0.x.y you can basically do anything. Nothing is stable and the public API could be changed at any time. Sure, a lot of projects are probably using it right now, but I guess there is a reason you still keep at 0.x.y.

Best regards,
Ludwig

@daogrady
Copy link
Contributor

Hi Ludwig,

yes, that is correct. 🙂
Still, it has led to confusion in the past, so I am trying to introduce breaking changes sparingly, if possible.

Best,
Daniel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working keepalive Will not be closed by Stale bot
Projects
None yet
Development

No branches or pull requests

3 participants