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

[docs] Add TypeScript demos for Simple and Spanning Table #14985

Merged
merged 7 commits into from
Mar 23, 2019
Merged

[docs] Add TypeScript demos for Simple and Spanning Table #14985

merged 7 commits into from
Mar 23, 2019

Conversation

jasondashwang
Copy link
Contributor

@jasondashwang jasondashwang commented Mar 21, 2019

This is my first time contributing to this project - before I continue adding TS demos, I want to make sure I am doing it correctly with my first two examples - SimpleTable and SpanningTable.

@jasondashwang
Copy link
Contributor Author

I don't know what to do with spread operator in SpanningTable.tsx because TypeScript considers the spread operator incompatible with the function signature of createRow.

Source:
microsoft/TypeScript#4130

@oliviertassinari oliviertassinari changed the title added doc types for SimpleTable and SpanningTable [Table] Migrate some demos to TypeScript Mar 21, 2019
@oliviertassinari oliviertassinari added component: table This is the name of the generic UI component, not the React module! typescript labels Mar 21, 2019
@oliviertassinari
Copy link
Member

@hopelessmuffins Have made a new commit. Let me know what you think of the simplification proposal.

@mui-pr-bot
Copy link

mui-pr-bot commented Mar 21, 2019

No bundle size changes comparing d1a7d76...ae3fc37

Generated by 🚫 dangerJS against ae3fc37

@eps1lon eps1lon self-requested a review March 21, 2019 08:47
@jasondashwang
Copy link
Contributor Author

Yep that works - I was wanting to change the .js version of the file, but I rather run it by you first. Thanks - will start working on the rest.

}

function subtotal(items) {
return items.map(({ price }) => price).reduce((sum, i) => sum + i, 0);
}

const rows = [
['Paperclips (Box)', 100, 1.15],
['Paper (Case)', 10, 45.99],
Copy link
Member

Choose a reason for hiding this comment

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

I don't know what to do with spread operator in SpanningTable.tsx because TypeScript considers the spread operator incompatible with the function signature of createRow.

Source:
Microsoft/TypeScript#4130

This was an issue with type widening. TypeScript inferred a nested array (string | number)[][] instead of an array of tuples [string, number, number][]. With TypeScript 3.4 we could've written ['Paper (Case)', 10, 45.99] as const to fix this issue.

function createData(name, calories, fat, carbs, protein) {
id += 1;
Copy link
Member

Choose a reason for hiding this comment

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

That's a good change. The previous approach resulted in re-mounting of the TableRow on every render call of SimpleTable.

@eps1lon eps1lon mentioned this pull request Mar 23, 2019
@eps1lon eps1lon changed the title [Table] Migrate some demos to TypeScript [Table] Add TypeScript demos for Simple and Spanning Table Mar 23, 2019
@eps1lon eps1lon changed the title [Table] Add TypeScript demos for Simple and Spanning Table [docs] Add TypeScript demos for Simple and Spanning Table Mar 23, 2019
@eps1lon eps1lon added docs Improvements or additions to the documentation and removed component: table This is the name of the generic UI component, not the React module! labels Mar 23, 2019
@eps1lon eps1lon merged commit e8ecc8f into mui:next Mar 23, 2019
@eps1lon
Copy link
Member

eps1lon commented Mar 23, 2019

@jasondashwang Much appreciated, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants