impero-com / typebinder

Exports TS definitions from Rust module
Other
20 stars 2 forks source link

Unnecessary quoting for property names #23

Closed gobanos closed 3 years ago

gobanos commented 3 years ago
#[derive(Serialize, Deserialize)]
pub struct Foo {
    delete: bool,
}

Typebinder generates an interface with unnecessary quoting around property name :

interface Foo {
    "delete": boolean
}

I think we should only factorize the regex check if TSIdent and not the keyword check.

greyblake commented 3 years ago

I was asked to take a look at this one buy Oliver. While it's easy to address (remove RESERVED check in ts_json_subset/src/ident.rs) am not sure it's good.

From ts_json_subset perspective I'd argue that this change is not legit, because idents (theoretically) can be used widely outside interfaces, where they will clash with TS keywords.

On other hand if we think about it only v scope of typebinder, then the change is OK.

I personal preference would be to leave it as it's (corrected expected behaviour over prettiness). But I want to let @AlisCode speak :)

AlisCode commented 3 years ago

Maybe the cleanest way is to have an UncheckedTsIdent but for the scope of typebinder it's fine to just forget about reserved keywords I guess. As long as the output compiles fine even when using reserved keywords ...

gobanos commented 3 years ago

Just to be clear, we still need to reject identifiers that are a reserved keyword, but not parameters. This is invalid :

interface delete {
    foo: number,
}