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

Conflict between Record<string, never> and Serde Tag in TypeScript Output #174

Closed
bennobuilder opened this issue Nov 22, 2023 · 4 comments
Closed

Comments

@bennobuilder
Copy link
Contributor

Description

In Specta, an empty struct is represented as Record<string, never> in the TypeScript output, which conflicts with the Serde tag = type pattern.
image

Suggested Change

It's suggested to modify the handling of empty structs in TypeScript output, particularly in cases where Serde's tag is involved. Instead of using {} in general (which is not a good practice as explained in Total TypeScript - The Empty Object Type in TypeScript), the solution might involve a conditional approach based on the presence of Serde's tag.

More context: https://discord.com/channels/949090953497567312/1176756946796298310/1176761059764351127

bennobuilder added a commit to bennobuilder/specta that referenced this issue Nov 22, 2023
@bennobuilder
Copy link
Contributor Author

bennobuilder commented Nov 22, 2023

@oscartbeaumont

Thanks for the quick action. Though for me the proposed fix didn't work :/

Logs

--- Named: NamedFields {
    fields: [],
    tag: None,
}
--- Named: NamedFields {
    fields: [],
    tag: None,
}

Logs from

  if fields.is_empty() {
                println!("--- Named: {:#?}", s);
                return Ok(s
                    .tag()
                    .as_ref()
                    .map(|tag| format!("{{ \"{tag}\": \"{key}\" }}"))
                    .unwrap_or_else(|| format!("Record<{STRING}, {NEVER}>")));
            }

Code

#[derive(Debug, Deserialize, Type, Clone)]
#[serde(tag = "type")]
pub enum InteractionInputEvent {
    CursorEnteredComposition(CursorEnteredComposition),
    CursorExitedComposition(CursorExitedComposition),
    // ..
}

// ..

#[derive(Event, Debug, Deserialize, Type, Clone)]
pub struct CursorEnteredComposition {} // TODO: Change back to Unit struct when https://github.com/oscartbeaumont/specta/issues/174 fixed

#[derive(Event, Debug, Deserialize, Type, Clone)]
pub struct CursorExitedComposition {} // TODO: Change back to Unit struct when https://github.com/oscartbeaumont/specta/issues/174 fixed

// ..

Generated types:

export type CursorEnteredComposition = Record<string, never>

export type CursorExitedComposition = Record<string, never>

export type InteractionInputEvent = ({ type: "CursorEnteredComposition" } & CursorEnteredComposition) | ({ type: "CursorExitedComposition" } & CursorExitedComposition)

Experiment with Unit

Also it would be nice to support serde tags for Unit structs too :) (if possible)

  StructFields::Unit => {
            println!("--- Unit: {:#?}", s);
            Ok(s.tag()
                .as_ref()
                .map(|tag| format!("{{ \"{tag}\": \"{key}\" }}"))
                .unwrap_or_else(|| NULL.into())) // TODO
        }

Logs

--- Unit: StructType {
    name: "CursorEnteredComposition",
    generics: [],
    fields: Unit,
}
--- Unit: StructType {
    name: "CursorExitedComposition",
    generics: [],
    fields: Unit,
}

Thanks :)

@oscartbeaumont
Copy link
Member

Ohhhh, now I get you.

Checking with what Serde do the expected behavior should be :

pub struct Inner {}

Previously: { "tag": "tagValue" } & Record<string, never>
Expected: { "tag":"tagValue" }

pub struct Inner

Previously: { "tag": "tagValue" } & null which Typescript coalesces to never.
Expected: { "tag":"tagValue" }

It looks like we already have a check related to this in place but it's not working correctly with references. Eg. it checks for the string null but in this case it would be Inner so the check fails.

pub struct Inner()

Previously: { "tag": "tagValue" } & []
Expected: Invalid Type Error

@oscartbeaumont
Copy link
Member

Okay give 467e2b0 a shot and lemme know if you still are having issues.

@bennobuilder
Copy link
Contributor Author

Okay give 467e2b0 a shot and lemme know if you still are having issues.

Seems to work as expected :)

Thanks a lot for the quick fixes 👍 🙏

export type InteractionInputEvent = ({ type: "CursorDownOnEntity" } & CursorDownOnEntity) | ({ type: "CursorMovedOnComposition" } & CursorMovedOnComposition) | ({ type: "CursorEnteredComposition" }) | ({ type: "CursorExitedComposition" })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants