Templarian / MaterialDesign-React

Dist for Material Design Icons React Component for JS/TypeScript
https://materialdesignicons.com
MIT License
142 stars 20 forks source link

ToDo: TypeScript #1

Closed Templarian closed 6 years ago

Templarian commented 6 years ago

Rewrite this component in TypeScript 3 and include the definition files like we do for @mdi/js.

azdanov commented 6 years ago

Are PR's welcome for this?

Templarian commented 6 years ago

@azdanov Usually I would say yes in a heartbeat, but I have it almost done. Definitely can look it over and make tweaks in a PR once I have it merged. 😄

Templarian commented 6 years ago

@azdanov https://github.com/Templarian/MaterialDesign-React/blob/master/src/Icon.tsx

Not even tested or compiled yet. Just committing a rough draft. It's really close to your PR. Busy tonight working on some other stuff, but will have more time this weekend.

Templarian commented 6 years ago

ToDo: Need to add the types definition file to the package.json.

Templarian commented 6 years ago

@azdanov Okay, so I've ran into a really dumb bug/issue. I can't get the propTypes to validate in a test project. VS Code clearly shows that they are required or not, but it doesn't throw an error in the browser in `mode=development'.

Basic example: <Icon /> Since path={} is required it should be throwing an error.

The propTypes are showing up in the compiled code. Tried to publish as dev for the Icon component it shows errors, but production removes errors. I assumed if the parent project was in dev that component would still report errors.

Kind of giving up for now and moving on to adding icons and will look into this shortly. Any help would be appreciated. Attached my sample app.

jsreact.zip .

azdanov commented 6 years ago

Add this to webpack.config.js:

externals: {
  react: "commonjs react",
  "prop-types": "commonjs prop-types"
}
Templarian commented 6 years ago

That worked. Awesome!

azdanov commented 6 years ago

Probably same issue as React. Facebook still uses commonjs.

azdanov commented 6 years ago

I'm wondering if webpack is needed. Since this is used in react, and probably with a build system (who still writes createElement?), then distributing es6 code is fine.

By itself tsc will transform typescript to javascript, and the rest is for the end-user to manage.

EDIT:

Forgot to mention some benefits:

Templarian commented 6 years ago

Yea, the @mdi/js only uses tsc. Considered simplifying this build also. We only need an ES6 module for this lib, so probably a good idea.

Templarian commented 6 years ago

Closing this and working on updating the build to use TSC with a new issue.

@azdanov Can you do a PR and add a contributors property to the package.json for yourself since you co-wrote this library.