Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Enable support for typed children using TSX #682

Closed
wants to merge 1 commit into from
Closed

Enable support for typed children using TSX #682

wants to merge 1 commit into from

Conversation

bitpshr
Copy link
Member

@bitpshr bitpshr commented Sep 14, 2017

Type: feature

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

This PR adds the ElementChildrenAttribute interface inside the jsx namespace to enable typed children.

Resolves #675

Copy link
Contributor

@matt-gadd matt-gadd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bitpshr do you think we could get a typings test for this? might be worth building it off @agubler's PR #678 which fixes the TSX integration tests we'd neglected

@kitsonk
Copy link
Member

kitsonk commented Sep 15, 2017

Not 100% sure if it is relevant in this one, but we do have a rudimentary type assertions test tool over inter dojo/interfaces: https://github.com/dojo/interfaces/blob/master/tests/support/assertType.ts

@bitpshr
Copy link
Member Author

bitpshr commented Sep 15, 2017

@matt-gadd how could I test this properly? I have something like:

	'allows for typed children'() {
		class Bar extends WidgetBase { }
		class Foo extends WidgetBase<WidgetProperties, WNode<Bar>> {
			render() {
				return <Bar />;
			}
		}
	}

But since widget-core doesn't do runtime checks on children, using something like throws and adding an incorrect child inside Foo wouldn't actually throw. How can I test a compile-time type check?

@matt-gadd
Copy link
Contributor

@bitpshr given #675, i'm going to go ahead and close this - sorry for wasting your time!

@matt-gadd matt-gadd closed this Sep 19, 2017
@dylans dylans added this to the 2017.09 milestone Sep 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants