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

Extensible records #5715

Closed
wants to merge 0 commits into from
Closed

Extensible records #5715

wants to merge 0 commits into from

Conversation

cristianoc
Copy link
Collaborator

@cristianoc cristianoc commented Oct 2, 2022

Add support for extensible records of the form e.g.:

type t = {...t1, x:int, ...t2}

Fields after expansion must not be repeated.

See #5659

@cristianoc cristianoc marked this pull request as draft October 2, 2022 12:12
@cristianoc
Copy link
Collaborator Author

@mattdamon108 take a look and ignore syntax and the rough state. Curious if this mechanism is enough to represent concisely what's required to have different types for different Dom elements. How about creating a little mock like the example in the PR, without putting all the fields only some to test the idea, see how it looks like?

@cannorin possibly relevant for conversion of TS bindings.

CC @zth

@zth
Copy link
Collaborator

zth commented Oct 2, 2022

I like this! This is going to be helpful in many cases.

@mununki
Copy link
Member

mununki commented Oct 2, 2022

How about creating a little mock like the example in the PR, without putting all the fields only some to test the idea, see how it looks like?

Got it. I’ll make some example for it. The sample cases would be the representation of types for dom props and dom events.

@mununki
Copy link
Member

mununki commented Oct 2, 2022

If I read the code correctly, using label as “dotdotdot” means the single extension, right? Not like {…a, …b}

@cristianoc
Copy link
Collaborator Author

cristianoc commented Oct 2, 2022

If I read the code correctly, using label as “dotdotdot” means the single extension, right? Not like {…a, …b}

Correct. Only one extension at the moment.
Curious if more are required. Can be generalized in that case.

@mununki
Copy link
Member

mununki commented Oct 2, 2022

If I read the code correctly, using label as “dotdotdot” means the single extension, right? Not like {…a, …b}

Correct. Only one extension at the moment. Curious if more are required. Can be generalized in that case.

Maybe or Maybe not. Not sure for now, but I think the single extension seems enough for the dom definition at this moment.

@mununki
Copy link
Member

mununki commented Oct 3, 2022

added some example of record extension for dom elements
410f3c2

@cristianoc
Copy link
Collaborator Author

Added support for multiple "dotdotdot"

@cristianoc
Copy link
Collaborator Author

Some design questions about fields possibly repeating.

This is now possible:

  type t1 = {x: int}
  type t2 = {dotdotdot: t1, x: string}

What it creates, is a type that cannot be used, as it contains x twice.

Two possible ways are:

  1. give an error
  2. make t2 override the definition in t1

@cristianoc cristianoc force-pushed the extensible_records_poc branch from 342d0f6 to 2034d8e Compare April 12, 2023 08:17
@cristianoc cristianoc marked this pull request as ready for review April 12, 2023 08:30
@cannorin
Copy link
Contributor

@cristianoc From the code generation perspective, it is probably better to make t2 override it (that needs less corner-case checking).

@cristianoc
Copy link
Collaborator Author

@cristianoc From the code generation perspective, it is probably better to make t2 override it (that needs less corner-case checking).

We can always relax that later if needed (without causing braking changes).
While if we start permissive it's more difficult to revert.

@cannorin
Copy link
Contributor

We can always relax that later if needed (without causing braking changes).
While if we start permissive it's more difficult to revert.

We are going to encounter this corner case very often because many TS users write something like this:

interface A {
  foo: any;
  ...
}

interface B extends A {
  foo: string;
}

So I guess it is better to start permissive in this case?

@cristianoc
Copy link
Collaborator Author

We can always relax that later if needed (without causing braking changes).
While if we start permissive it's more difficult to revert.

We are going to encounter this corner case very often because many TS users write something like this:

interface A {
  foo: any;
  ...
}

interface B extends A {
  foo: string;
}

So I guess it is better to start permissive in this case?

The tooling can get special capabilities. In fact it could even live inside the compiler, so it's easier to iterate on these things in a PR.
This PR is for direct users.

@cristianoc
Copy link
Collaborator Author

cristianoc commented Apr 12, 2023

About surface syntax, this is perfectly valid object type definition, and trying to support ... for records would simply be ambiguous.

type xy = {
  "x": float,
  "y": float,
}
type z = {
  "z": float,
}

type xyz = {
  ...xy,
  ...z,
}

@cristianoc cristianoc changed the title Proof of concept for extensible records Extensible records Apr 13, 2023
@cristianoc cristianoc requested a review from zth April 13, 2023 11:40
@cristianoc cristianoc closed this Apr 13, 2023
@cristianoc cristianoc force-pushed the extensible_records_poc branch from 04c3010 to be88572 Compare April 13, 2023 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants