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

[typescript] Add tests for typings + fixes #7686

Merged
merged 46 commits into from Aug 15, 2017

Conversation

sebald
Copy link
Member

@sebald sebald commented Aug 7, 2017

This patch will add testing for TS typings (finally) and fixes some wrong typings (details below).

Closes #7696

  • Ship the typings with the build
  • Run the tests yarn run test:typings [-- --watch]
  • Add typings for GridList
  • Add typings for transitions Collapse, Fade and Slide
  • Fix TestFieldProps (typo...)
  • Fix SnackbarProps (action can be an array, message can be a string)
  • Fix TransitionProps (duration can be a string)
  • Fix FormGroupProps (row is optional)
  • Fix SlideProps (direction + offset where missing)
  • Use SlideProps in DrawerProps

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

skirunman and others added 30 commits August 2, 2017 16:48
* Fix language issues for clarity

Fix up a bunch of English language usage issues and try to provide more clarity

* Fix typo
A value of baseline is not provided for the implementation of align-items in the Grid component [issue mui#7620]:
- Rebuilt Grid docs
- Added prop type value of baseline to align
- Added align-xs-baseline style to Grid stylesheet
- Added baseline to interactive grid demo
* upstream/v1-beta:
  [core] add isMuiComponent helper (mui#7635)
  yank invalid `indeterminate` props copied to Switch and Radio
  modify test for change in referential transparency
  [flow] missed type conversion
  [docs] build
  [flow] type switch, radio, checkbox, textarea, input and related.
  [flow] update package
  [Grid] Add baseline to container's align property (mui#7623)
  [docs] Show how to use the insertionPoint (mui#7611)
  [Docs] Fix language issues for clarity (mui#7610)
* [docs] Update RadioGroups onChange prop.
* Fixed comments typo

* Fixed popular misspelled words

* Fixed some more spelling mistakes

* Fixed 2 more words

* Update AppBar.js

* Update AppBar.js
* [GridList] migrate to next branch

* [Grid] Rename gutter into spacing
* [ListItemText] Repurpose text class
- Expose text as a class that can be used to style the inner Typography component, whether the List is dense or not
- Add new textDense class that can be used to specifically style the Typography component when the list is dense
- Reference to closed issue mui#7067

* Update ListItemText.js
* Eslint config for spell check

* [Eslint] only spelling test

* [Typo] fixed tests
* [flow] export type Props
* [flow] `classes` is optional as an external prop, but always present inside a withStyles enhanced component
* [flow] introduce `AllProps` type and remove intersection from externalized Props type

External use of exported `type Props` was hindered as it was an intersection that mixed in the `DefaultProps`.  This prevented external use of composite types when using $Exact<Props> or {|Props|}.  This commit keeps everyting in isolation and exported Props are now pure.
* babel rc option for optimization
* [Typography] fix prop vs default prop headlineMapping
* upgrade babel-plugin-transform-react-remove-prop-types and flow
* [ButtonBase] className is optional
* [Table] Take advantage of the indeterminate in the advanced demo

* [Portal] Better support of react@next
* [typescript] Make `theme` optional for <Drawer>.

* [Docs] Fix language issues for clarity (mui#7610)

* Fix language issues for clarity

Fix up a bunch of English language usage issues and try to provide more clarity

* Fix typo

* [docs] Show how to use the insertionPoint (mui#7611)

* [typescript] Re-add @types

* [typescript] Add props for modal/portal

* [Grid] Add baseline to container's align property (mui#7623)

A value of baseline is not provided for the implementation of align-items in the Grid component [issue mui#7620]:
- Rebuilt Grid docs
- Added prop type value of baseline to align
- Added align-xs-baseline style to Grid stylesheet
- Added baseline to interactive grid demo

* [flow] update package

* [flow] type switch, radio, checkbox, textarea, input and related.

* [docs] build

* [flow] missed type conversion

* modify test for change in referential transparency

* yank invalid `indeterminate` props copied to Switch and Radio

* [typescript] Correct props for <Modal> + <Drawer>

* [typescript] Correct typings for `<RadioGroup>` change handler.

* [typescript] Make Color less strict.

* [typescript] Tab(s) inherit from ButtonBase/HTMLDiv

* [typescript] Table* inherits from their "native" counterparts.

* [typescript] Correct Breakpoints typing.

* [typescript] Add props from {...other} -> <CardMedia>

* [typescript] Add SwitchBase typings

* [typings] Inherit props from native elemnts <Form

* [typescript] Finish adding inherited props from {...other}

* [typescript] Make TransitionHandlers optional

* [typescript] Improve "ref" typings.

* [typescript] Move @types to `devDependencies`.

* [typescript] Fix missing inheritance of MenuItem

* [typescript] Compose instead of inherit.

* [typescript] Add Diff/Omit helper to solve issues with `title` prop.
@stevegeek
Copy link

stevegeek commented Aug 10, 2017

@sebald Is it just me or is Collapse, Slide and Fade transition missing from defs?

declare module 'material-ui/transitions/Collapse' {
  import {TransitionCallback} from 'material-ui/internal/Transition';
  interface CollapseProps {
    in?: boolean;
    transitionDuration?: number | 'auto';
    onEnter?: TransitionCallback;
    onEntered?: TransitionCallback;
    onEntering?: TransitionCallback;
    onExit?: TransitionCallback;
    onExited?: TransitionCallback;
    onExiting?: TransitionCallback;
  }
  export default class Collapse extends MaterialUI.Component<CollapseProps> {}
}


declare module 'material-ui/transitions/Slide' {
  import {TransitionCallback} from 'material-ui/internal/Transition';
  interface SlideProps {
    direction?: 'left' | 'right' | 'up' | 'down';
    enterTransitionDuration?: number;
    in?: boolean;
    leaveTransitionDuration?: number;
    offset?: string;
    onEnter?: TransitionCallback;
    onEntered?: TransitionCallback;
    onEntering?: TransitionCallback;
    onExit?: TransitionCallback;
    onExited?: TransitionCallback;
    onExiting?: TransitionCallback;
  }
  export default class Slide extends MaterialUI.Component<SlideProps> {}
}

declare module 'material-ui/transitions/Fade' {
  import {TransitionCallback} from 'material-ui/internal/Transition';
  interface FadeProps {
    enterTransitionDuration?: number;
    in?: boolean;
    leaveTransitionDuration?: number;
    offset?: string;
    onEnter?: TransitionCallback;
    onEntered?: TransitionCallback;
    onEntering?: TransitionCallback;
    onExit?: TransitionCallback;
    onExited?: TransitionCallback;
    onExiting?: TransitionCallback;
  }
  export default class Fade extends MaterialUI.Component<FadeProps> {}
}

@sebald
Copy link
Member Author

sebald commented Aug 10, 2017

@stevegeek Which version of the typings are you using? I added them in 99c5658

@stevegeek
Copy link

@sebald Ah ok I must be using one from before that commit! thanks!

@sebald
Copy link
Member Author

sebald commented Aug 10, 2017

@stevegeek You're very welcome :) Let me know if you run into problems with them!

@stevegeek
Copy link

@sebald I think direction and offset are missing from Slide https://github.com/callemall/material-ui/blob/v1-beta/src/transitions/Slide.js#L35

  direction?: 'left' | 'right' | 'up' | 'down';
  offset?: string;

Also enterTransitionDuration and leaveTransitionDuration on Slide and Fade are just number , not number | 'auto' like transitionDuration on Collapse (see https://github.com/callemall/material-ui/blob/v1-beta/src/transitions/Fade.js#L26 vs https://github.com/callemall/material-ui/blob/v1-beta/src/transitions/Collapse.js#L75)

@stevegeek
Copy link

Another question, for material-ui-icons , what do you tihnk about adding typings to that package too?

eg

// Either declare here or import SvgIconProps from main typings as all icons are implemented with SvgIcon
declare interface IconProps extends React.HTMLAttributes<{}> {
  titleAccess?: string;
  viewBox?: string;
}

declare module 'material-ui-icons/Menu' {
  export default class Menu extends React.Component<IconProps> {}
}

declare module 'material-ui-icons/ExitToApp' {
  export default class Icon extends React.Component<IconProps> {}
}

... etc for all icons. Here could write a script to generate the declarations from the files names in https://github.com/callemall/material-ui/tree/v1-beta/packages/material-ui-icons/src

Sebastian Sebald added 2 commits August 10, 2017 10:54
- direction + offset where missing
- use SlideProps in DrawerProps
@sebald
Copy link
Member Author

sebald commented Aug 10, 2017

@stevegeek Thanks. Updated!

Yes, I want to add typigns for the icons, too. I just didn't have time yet. Which is kinda dump to say, because we already have typings internally ;D

We're using the following script to generate the typings:

/**
 * Generate type definitions for `material-ui-icons`.
 * The generated contens will be written to
 * `src/typings/material-ui-icons.d.ts`!
 */
const { parse, resolve } = require('path');
const { EOL } = require('os');
const chalk = require('chalk');
const { writeFile } = require('fs-extra');
const globby = require('globby');
const { map, pipe } = require('ramda');
const { error } = require('./utils');
const log = console.log;

const MODULE_DIR = resolve(__dirname, '../node_modules/material-ui-icons');
const DTS_FILE = resolve(__dirname, '../src/typings/material-ui-icons.d.ts');

// Helpers
// ---------------
const normalizeFileName = name => parse(name).name;

const createTypeDefinition = name =>
  `declare module 'material-ui-icons/${name}' {
  import SvgIcon from 'material-ui/SvgIcon';
  export default class ${name} extends SvgIcon {}
}`;

const convertFilesToTypings = map(
  pipe(normalizeFileName, createTypeDefinition)
);

// Script
// ---------------
log(`\u{1f52c}  Searching for modules inside "${chalk.dim(MODULE_DIR)}".`);
globby(['*.js', '!index*'], { cwd: MODULE_DIR })
  .then(convertFilesToTypings)
  .then(typings => typings.join(`${EOL}${EOL}`))
  .then(contents => writeFile(DTS_FILE, contents, { encoding: 'utf8' }))
  .then(() => log(`\u{1F5C4}  Written typings to ${chalk.dim(DTS_FILE)}.`))
  .catch(err => {
    if (err) {
      error(err);
    }
    process.exit(1);
  });

Just change the MODULE_DIR and DTS_FILE and you should be good to go.

@sebald
Copy link
Member Author

sebald commented Aug 10, 2017

Your output should look something like this: https://gist.github.com/sebald/c150a718aa05b2206dbdf1ef9e07e9d2

@stevegeek
Copy link

@sebald Thats just perfect, thanks!!

* upstream/v1-beta: (24 commits)
  v1.0.0-beta.5
  Update CHANGELOG.md
  [CHANGELOG] Prepare v1.0.0-beta.5
  chore(package): update size-limit to version 0.9.0 (mui#7757)
  [docs] Fix yarn docs:build script (mui#7745)
  [docs] Update the readme
  [Radio] Fix accessibility issue (mui#7742)
  [Tabs][BottomNavigation] Use value over index property (mui#7741)
  [core] Remove createStyleSheet (mui#7740)
  [core] Start simplifying styling API (mui#7730)
  [LinearProgress] Use transform instead width (mui#7732)
  Fix Typo (mui#7736)
  Documentation Fix - Fixing up the README documentation (mui#7733)
  [ButtonBase] Expose internal component (mui#7727)
  [Popover] Refactor popover transition - separation of concerns (mui#7720)
  Button documentation fix (mui#7726)
  Update supported-components.md (mui#7722)
  chore(package): update webpack to version 3.5.3 (mui#7723)
  [flow] flow type transitions Slide, Fade, Collapse (fixes)
  Create CODE_OF_CONDUCT.md
  ...
@oliviertassinari
Copy link
Member

Oops, I have forgotten to merge the PR before the latest release. I will give you write access so we can work with more efficiency.

@sebald
Copy link
Member Author

sebald commented Aug 15, 2017

@oliviertassinari ¯_(ツ)_/¯ happens!

Wow, thanks :-O now I am scared.

@oliviertassinari
Copy link
Member

Don't worry, it's gonna be all right. Whenever you want to improve the typescript coverage, go ahead. I don't know anyone better than you to take care of it.

@sebald sebald merged commit babfdb4 into mui:v1-beta Aug 15, 2017
@sebald sebald deleted the setup-typings-tests branch August 15, 2017 11:29
@marcusjwhelan
Copy link

Will there be types for Grid? @sebald

@sebald
Copy link
Member Author

sebald commented Aug 16, 2017

@marcusjwhelan Not sure if I can follow. Do you have problems using them? Because there already are typings for Grid:

https://github.com/callemall/material-ui/blob/babfdb4e7ec6b158e7b1d16a5576f4efdfcf14ac/typings/index.d.ts#L587-L630

Please let me know if you run into issues with using the typings.

@marcusjwhelan
Copy link

marcusjwhelan commented Aug 16, 2017

@sebald @oliviertassinari The Typings are not shipped with npm. I have tried next. There is the @types/material-ui but it is only up to date with 0.17.

@oliviertassinari
Copy link
Member

We need to release these typings! I can do a quick release before the end of the week.

@stevegeek
Copy link

@marcusjwhelan I have been using these typings for a while now and they are great so once released you can rely on them

@sebald @oliviertassinari Before releasing can we make sure to add the changes for the new withStyles and the removed createStyleSheet method

@sebald
Copy link
Member Author

sebald commented Aug 16, 2017

@stevegeek Can you please open an issue? (maybe even elaborate on the change)

@vyrotek
Copy link

vyrotek commented Aug 16, 2017

@sebald I believe @stevegeek is referring to #7740 and #7730 which were just released in beta 5

I'm definitely looking forward to the typings being released!

@stevegeek
Copy link

@sebald @vyrotek Sorry just saw this message, yeah was referring to those latest changes in the API, you would like me to open an issue?

@sebald
Copy link
Member Author

sebald commented Aug 17, 2017

@stevegeek See #7793 :)

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.

None yet