search

eiriklv / react-masonry-component

A React.js component for using @desandro's Masonry
MIT License
1.44k stars 145 forks source link

Fix wrong default export in TypeScript typings #52

Closed rasmuskl closed 8 years ago

rasmuskl commented 8 years ago

Fixed TypeScript typings.

Exporting as default actually creates an export named default. The JS code for the react-masonry-component doesn't do this, but sets module.export = the output from React.createClass, which is of type ComponentClass<PropType>.

When using the old typings as a default import in TypeScript:

import Masonry from "react-masonry-component";

You get this error on runtime:

warning.js:36 Warning: React.createElement: type should not be null, undefined, boolean, or number. It should be a string (for DOM elements) or a ReactClass (for composite components). Check the render method of `ExplorerView`.
invariant.js:38 Uncaught Invariant Violation: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. Check the render method of `ExplorerView`.

With the new typings, importing like this (which is the TypeScript equivalent of the export from the module):

import * as Masonry from "react-masonry-component";

Works with no error.

afram commented 8 years ago

Thanks for this - I wonder, would it be better to add this line to the index.js component file instead?

module.exports.default = MasonryComponent;

It seems a bit odd to me to import * when react-masonry-component is only a single export. This also has the added benefit of being backwards compatible.

I'm not too knowledgable on TypeScript though, so please let me know if I've missed something :-)

rasmuskl commented 8 years ago

That's a great idea. I didn't want to intrude on how you do your exports, but it would feel more natural the other way for sure.

I made the changes locally, verified that everything still works and updated the pull request.

Thanks for the quick feedback.

rasmuskl commented 8 years ago

My import now looks like this with the latest changes:

import Masonry from "react-masonry-component";
rasmuskl commented 8 years ago

@afram Any chance of a new release with the changes? :-)

afram commented 8 years ago

yeah, I was holding up to see if it is still compatible with the original TypeScript definitions. This version uses ComponentClass (which from what I can tell seems to be the base component) so should be OK.

afram commented 8 years ago

This is released as 4.2.2

Sorry for the delay.

rasmuskl commented 8 years ago

No problem. Thanks!