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

JSX: a way to make all the properties of value-based elements optional #7004

Open
thorn0 opened this issue Feb 10, 2016 · 23 comments
Open

JSX: a way to make all the properties of value-based elements optional #7004

thorn0 opened this issue Feb 10, 2016 · 23 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@thorn0
Copy link

thorn0 commented Feb 10, 2016

It's not documented, but currently, if the JSX.ElementAttributesProperty interface is declared empty,

declare namespace JSX { interface ElementAttributesProperty {} }

then the element attributes type becomes same as the element instance type. However, if the element instance type is a class, this leads to a situation where the compiler requires that all the members of the class be represented as attributes since they're not optional (and even cannot be marked as such as the class syntax doesn't allow optional members).

class MyComponent {
  myProp: string;
  private myMethod(): void;
}
var a = <MyComponent myProp='value'/>;
// error: Property 'myMethod' is missing in type MyComponent

This prevents me from using JSX for type-checking Angular 1 templates (you can find my experiments in this area here).

What if we introduce some way to tell the compiler that not all the properties are required, but only some of them? E.g. those having a certain decorator?

A related issue: #5151

@mhegazy mhegazy added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Feb 19, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Feb 19, 2016

So what is the proposal?

@RyanCavanaugh
Copy link
Member

I think we could reasonably say that private / protected members get ignored when checking against the element attributes type.

@thorn0
Copy link
Author

thorn0 commented Feb 20, 2016

Probably something like

declare namespace JSX { interface ElementOptionalAttributesProperty {} }

All the properties would be considered optional. Not ideal but could be a good start.

Ideally, I'd like to use decorators to mark required properties, like @Input and @Output in Angular 2. But I can't think of a good way to tell the compiler about such rules.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 9, 2016

Discussed at the backlog slog today. There are a lot of scenarios here that would be helped by this, but the technical implications of the first type operator are very large. Some more notes are at #8228. We're continuing to think about it.

Just in terms of setting expectations, this isn't going to be something we can do in time for TypeScript 2.0

@vvakame
Copy link
Contributor

vvakame commented Nov 4, 2016

I opened duplicated issue #12016 .
I wrote my motivation to that issue.

I read backlog slog. I think I do not want a big thing to this issue.

My proposal is below.

declare global {
    namespace JSX {
        // We use magic interface. this is pragma for compiler.
        interface ElementAttributesProperty {
            // use optional property
            ''?: any;
        }
    }
}

in src/compiler/checker.ts,

        function checkJsxOpeningLikeElement(node: JsxOpeningLikeElement) {

            ...

            // add makeOptionalProperty condition from ElementAttributesProperty interface
            if (targetAttributesType && !sawSpreadedAny && !makeOptionalProperty) {
                ...
            }

@vvakame
Copy link
Contributor

vvakame commented Nov 4, 2016

I think we could reasonably say that private / protected members get ignored when checking against the element attributes type.

This is not so good for skate.js.
skate.Component extends HTMLElement signature can't control modifier by myself.

@thorn0
Copy link
Author

thorn0 commented Nov 4, 2016

Yes, what I requested was just an alternative way to type check JSX, not 'optionalized types`, which is a separate complex feature.

@aluanhaddad
Copy link
Contributor

Partial types, as proposed by @RyanCavanaugh in #11233, might well be able to model this.

@thorn0
Copy link
Author

thorn0 commented Nov 4, 2016

@aluanhaddad Partial and optionalized types are the same thing. Even though it sounds like overkill for this issue, it will work, however we'll need a way to tell the compiler that <Foo ... /> refers to subset Foo not just Foo. So a new JSX-related setting will be needed.

And if we need a new switch anyway, does its implementation really need to involve partial types if it can be done without them like @vvakame wrote?

@vvakame
Copy link
Contributor

vvakame commented Nov 27, 2016

Now I am understood what @RyanCavanaugh and @aluanhaddad are said.
We can use Partial types (a.k.a Mapped types) now.

We need good proposal.

IMHO, If JSX.ElementAttributesProperty has callable property, Use the return type instead of the type passed as an argument for checking.
like this.

interface ElementAttributesProperty {
    ''<T>(v: T): Partial<T>;
}

This issue is last 2 mile of SkateJS+TypeScript. (other 1 is #12488 )
If you have no seen SkateJS yet, please try it once. it's fun!
http://skate.js.org/
https://github.com/vvakame/skatejs-todo/

@thorn0
Copy link
Author

thorn0 commented Nov 27, 2016

Hm...

abstract class Component {
  /** A bogus type-system-only property. */
  private __bogusProps: Partial<this>;
}

declare namespace JSX {
  type Element = string;
  interface ElementAttributesProperty {
    __bogusProps: Partial<this>;
  }
}

class FooComponent extends Component {
  foo: number;
  method() { }
}

// ta-da !
let z = <FooComponent foo={2}/>;

@EisenbergEffect
Copy link

@RyanCavanaugh It's been two years. Any progress on this front? We were hoping to enable JSX for Aurelia vNext but this issue is causing the compiler to be very unhappy with us.

@thorn0
Copy link
Author

thorn0 commented Jun 12, 2018

@EisenbergEffect Doesn't this solution work for you?

@EisenbergEffect
Copy link

One of the unique and important aspects of Aurelia's design is that it doesn't require using base classes for components. Components are plain classes. In fact, we discourage the use of inheritance and prefer guiding our community to use more compositional approaches. As such, even if the above did work, it wouldn't be an acceptable solution for us. That said, I've tried a few solutions that leverage the JSX namespace without inheritance and VS Code at least, doesn't seem to be affected at all in it's TS error reporting. It just doesn't work. I've been starting to wonder if there's a bug that was introduced in the last update... I need to experiment some more. It would be nice to have a real solution to this though.

thorn0 added a commit to thorn0/babel-plugin-transform-ng1-jsx that referenced this issue Oct 3, 2018
@thorn0
Copy link
Author

thorn0 commented Oct 3, 2018

@EisenbergEffect It suddenly crossed my mind that we could extend interface Object instead of a base class:

interface Object {
  /** A bogus type-system-only property. */
  __bogusProps: Partial<this>;
}

declare namespace JSX {
  type Element = string;
  interface ElementAttributesProperty {
    __bogusProps: any;
  }
}

But alas... TS adds the __bogusProps property to every object, but it thinks that this property has the type Partial<Object>, not Partial<...our more specific type...>.

BTW, I didn't quite understand your "even if the above did work". It does work.

@EisenbergEffect
Copy link

Thanks for continuing to explore this @thorn0 I honestly can't remember if I tried the technique above or not. I know I tried a few things which didn't seem to work at least at one time. What I wanted to emphasize was that, either way, it's contrary to Aurelia's philosophy to force the use of a base class, so we wouldn't want to recommend this approach to our community as a best practice.

@RyanCavanaugh What we really need is a way to declare that the element attributes are derived directly from the type itself. In other words the properties of the object are the attributes of the element. There's no special property involved.

@thorn0
Copy link
Author

thorn0 commented Oct 4, 2018

@EisenbergEffect Did you consider writing a design proposal?

@RyanCavanaugh
Copy link
Member

@EisenbergEffect that is supported already by declaring the type JSX.ElementAttributesProperty to have zero properties

@EisenbergEffect
Copy link

EisenbergEffect commented Oct 4, 2018

Here's my code:

import { createElement } from './templating/create-element';

declare namespace JSX {
  type Element = string;
  interface ElementAttributesProperty {
  }
}

class Foo {
  public bar: string;
}

const view = () =>
  <div>
    <Foo bar="baz"></Foo>
  </div>;

I get the error [ts] Property 'bar' does not exist on type '{}'. What have I done wrong?

@thorn0
Copy link
Author

thorn0 commented Oct 5, 2018

@EisenbergEffect Because of the import statement, your file is a module, which means your JSX namespace is local, not global.

To make it global, use declare global:

declare global {
  namespace JSX {
    type Element = string;
    interface ElementAttributesProperty {
    }
  }
}

But after that you're going to face my original issue: the compiler requires that all the non-optional members of the class (including methods and private members) be represented as attributes.

@EisenbergEffect
Copy link

Thanks @thorn0 ! I can confirm that exact behavior now. Here's my code:

import { createElement } from './templating/create-element';

declare global {
  namespace JSX {
    type Element = string;
    interface ElementAttributesProperty {
    }
  }
}

class Foo {
  public bar: string = '';
  public baz: number = 0;
}

const view = () =>
  <div>
    <Foo bar='test'></Foo>
  </div>;

@RyanCavanaugh This would be 100% valid in Aurelia but TS is complaining that the Foo element doesn't include a value for baz. It sounds like the TS checking for JSX scenarios is causing issues for both Angular and Aurelia. It would be nice to have a solution for this, some compiler setting or something that could be done with the ElementAttributesProperty declaration to indicate what behavior we need.

@china-liji
Copy link

china-liji commented Jun 14, 2019

Hi, @vvakame , the optional property is not working with typescript 3.4.5.

The typescript source code is here:
image

Now, i want to turn off required properties check in TSX context , what should i do?

@china-liji
Copy link

I got it that how to turn off the required properties check in TSX context.

The code is below:
jsx-namespace.d.ts

declare namespace JSX {
     // ...
    interface IntrinsicAttributes {
        [attributeName: string]: any
    }
    // ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants