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

tsx syntax support #475

Merged
merged 6 commits into from
May 6, 2017
Merged

tsx syntax support #475

merged 6 commits into from
May 6, 2017

Conversation

agubler
Copy link
Member

@agubler agubler commented May 5, 2017 •
edited
Loading

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:

Adds support for using tsx syntax to author Dojo 2 widgets within the render function.

return (
	<footer classes={this.classes(css.footer)}>
		<span classes={this.classes(css.todoCount)}>
			<strong>{`${activeCount}`}</strong>
			<span>{`${count}`}</span>
		</span>
		<TodoFilter activeFilter={activeFilter} />
		{ completedItems ? ( <button onclick={this.clearCompleted} /> ) : ( null ) }
	</footer>
);

Props to @matt-gadd for the tsx pragma and function implementation (the important parts!).

Resolves #469

@agubler agubler requested review from kitsonk, matt-gadd and rorticus May 5, 2017 15:08
rorticus
rorticus approved these changes May 5, 2017
View reviewed changes
Copy link
Contributor

@rorticus rorticus left a comment

Choose a reason for hiding this comment

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

Very cool. Think we should put anything in the README?

@dylans dylans added this to the 2017.05 milestone May 5, 2017
@codecov
Copy link

codecov bot commented May 5, 2017 •
edited
Loading

Codecov Report

Merging #475 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #475      +/-   ##
==========================================
+ Coverage   96.89%   96.96%   +0.07%     
==========================================
  Files          17       18       +1     
  Lines         869      891      +22     
  Branches      133      137       +4     
==========================================
+ Hits          842      864      +22     
  Misses          3        3              
  Partials       24       24
Impacted Files Coverage Δ
src/tsx.ts 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20022ee...48377a5. Read the comment docs.

@agubler
Copy link
Member Author

agubler commented May 5, 2017

@rorticus added some blurb on JSX in the README.

@agubler agubler requested a review from dylans May 5, 2017 16:51
@agubler agubler mentioned this pull request May 5, 2017
Closed
dylans
dylans suggested changes May 5, 2017
View reviewed changes
Copy link
Member

@dylans dylans left a comment

Choose a reason for hiding this comment

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

minor nits and suggestions

README.md Outdated

In additional to the programatic functions `v` and `w`, widget-core supports the use of the `jsx` syntax known as [`tsx`](https://www.typescriptlang.org/docs/handbook/jsx.html) in TypeScript.

To start to use `jsx` in your project the widgets need to be named with a `.tsx` extension and some configuration is required in the projects `tsconfig.json`:
Copy link
Member

Choose a reason for hiding this comment

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

projects -> project's

README.md Outdated
}
```

**Note:** Unfortunately `tsx` is not directly used so needs to be ignored by linters.
Copy link
Member

Choose a reason for hiding this comment

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

Not directly used with what? This could be clearer

README.md Outdated
@@ -173,6 +174,51 @@ w('my-widget', properties, children);
The example above that uses a string for the `widgetConstructor `, is taking advantage of the [widget registry](#widget-registry) functionality.
The widget registry allows for the lazy loading of widgets.

### tsx

In additional to the programatic functions `v` and `w`, widget-core supports the use of the `jsx` syntax known as [`tsx`](https://www.typescriptlang.org/docs/handbook/jsx.html) in TypeScript.
Copy link
Member

Choose a reason for hiding this comment

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

To be slightly more explicit that this is an alternative approach:

widget-core supports the use -> widget-core optionally supports the use

tests/unit/tsx.ts Outdated
assert.strictEqual((<any> RegistryWrapper).type, REGISTRY_ITEM);
const registryWrapper = new RegistryWrapper();
assert.strictEqual(registryWrapper.name, 'tag');
// These will always be undefined but show the type inference.
Copy link
Member

Choose a reason for hiding this comment

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

"show the type inference" ... I'd be more explicit, e.g. "show the type inference of ___________"

@agubler
Copy link
Member Author

agubler commented May 5, 2017

@dylans comments addressed

@agubler
Copy link
Member Author

agubler commented May 6, 2017

Merging as the CI failures are unrelated functional test failures.

@agubler agubler merged commit dfd4c66 into dojo:master May 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dylans dylans dylans approved these changes

@kitsonk kitsonk kitsonk approved these changes

@rorticus rorticus rorticus approved these changes

@matt-gadd matt-gadd Awaiting requested review from matt-gadd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2017.05
Development

Successfully merging this pull request may close these issues.
5 participants
@agubler @dylans @kitsonk @rorticus @matt-gadd