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 definition d.ts file? #42

Closed
empz opened this issue Jun 28, 2016 · 16 comments
Closed

TypeScript definition d.ts file? #42

empz opened this issue Jun 28, 2016 · 16 comments

Comments

@empz
Copy link

empz commented Jun 28, 2016

Would you consider including a TypeScript definition file in the package?

http://www.typescriptlang.org/docs/handbook/typings-for-npm-packages.html

@empz empz changed the title TypeScript defintion file? TypeScript definition d.ts file? Jun 28, 2016
@afram
Copy link
Collaborator

afram commented Jun 29, 2016

@emzero Yeah no issues with that

@afram
Copy link
Collaborator

afram commented Jun 29, 2016

@emzero do you feel like contributing one? 😛

@empz
Copy link
Author

empz commented Jun 29, 2016

@afram I'm actually pretty new to TypeScript and definitions so I did my bare minimum to make it work.

Here's what I got. I don't think a full PR is worth it, so here you go. Anyone is welcome to improve it.

declare module "react-masonry-component" {
  import React = __React;

  interface MasonryPropTypes {
    disableImagesLoaded: boolean;
    options: Object;
    className: string;
    elementType: string
  }

  export var Masonry: React.Component<MasonryPropTypes, void>;
}

@afram
Copy link
Collaborator

afram commented Jun 30, 2016

@emzero No worries.

I don't use TypeScript, please let me know how this beta version works for you. If it's all good I can release the next version.

npm i react-masonry-component@4.1.1-beta1 -S

@wjramos
Copy link

wjramos commented Jul 2, 2016

So, I am actually curious -- why TypeScript over React PropTypes?

MasonryComponent.propTypes = {
  disableImagesLoaded: PropTypes.bool,
  onImagesLoaded:      PropTypes.func,
  options:             PropTypes.object
};

See my fork for example:

https://github.com/wjramos/react-masonry-component/blob/masonry/components/MasonryComponent.js

@empz
Copy link
Author

empz commented Jul 2, 2016

@wjramos With TypeScript there's no need to use React PropTypes since a strongly-typed component using an interface to describe the props provides the same type checking at compile time.

Related

@empz
Copy link
Author

empz commented Jul 2, 2016

@afram That doesn't work because it depends on React typings being installed as a global module and might not be always the case. I'm also getting another error from Typescript.

TypeScript 2.0 is about to be released and it will drastically improve the way d.ts files are distributed for npm packages. So I would suggest that we just wait for it to include a proper d.ts file.

In the meantime, you could include a section in the readme saying something like...

If you are using TypeScript, include the following code in your custom definition file.

declare module "react-masonry-component" {
  import React = __React;

  interface MasonryPropTypes {
    disableImagesLoaded: boolean;
    options: Object;
    className: string;
    elementType: string
  }

  export var Masonry: React.Component<MasonryPropTypes, void>;
}

@wjramos
Copy link

wjramos commented Jul 3, 2016

@emzero wouldn't it be adding an otherwise unnecessary dependency?

@empz
Copy link
Author

empz commented Jul 3, 2016

@wjramos What do you mean?

EDIT: I think I understand what you're saying now.

I opened this issue to ask the author to include TypeScript definitions. This does not mean the library is going to depend on TypeScript. It's just a file that's needed if the library is going to be used in a TypeScript environment.

The original Javascript can still make use of PropTypes while the Typescript definition file uses an interface to define the props.

@afram
Copy link
Collaborator

afram commented Jul 3, 2016

@emzero I've updated the typings to import React directly. Can you please let me know if this works for you?

This was a quick "fix" based on an hour of reading, not sure if it's going to work.

Either way, you've motivated me to start taking TypeScript a bit more seriously.

react-masonry-component@4.1.1-beta3

@empz
Copy link
Author

empz commented Jul 3, 2016

The TypeScript compiler is complaining.

ERROR in node_modules\react-masonry-component\typings.d.ts (5,15):
error TS2665: Module augmentation cannot introduce new names in the top level scope.

ERROR in node_modules\react-masonry-component\typings.d.ts (14,16):
error TS2665: Module augmentation cannot introduce new names in the top level scope.

I actually have no idea how to make use of another external lib's typings. I've posted a question on SO but no answers yet.

As I said, TypeScript 2.0 will use npm to manage typings so it should provide better dependency management.

@afram
Copy link
Collaborator

afram commented Jul 7, 2016

@emzero I think I may have found a possible solution, can you please try this version? Appreciate your help in getting it out there :-)

react-masonry-component@4.1.1-beta4

@empz
Copy link
Author

empz commented Jul 9, 2016

@afram

I'm using typescript@next which is the 2.0 preview version for the next TS version that's just around the corner.
The following declaration file works great with this version. I wouldn't really bother to make it work with 1.8 today.

import { Component } from "react";

declare module "react-masonry-component" {

  export interface MasonryOptions {
    columnWidth?: number;
    itemSelector?: string;
    gutter?: number;
    percentPosition?: boolean;
    stamp?: string;
    fitWidth?: boolean;
    originLeft?: boolean;
    originTop?: boolean;
    containerStyle?: Object;
    transitionDuration?: string;
    resize?: boolean;
    initLayout?: boolean;
  }

  interface MasonryPropTypes {
    disableImagesLoaded?: boolean;
    updateOnEachImageLoad?: boolean;
    onImagesLoaded?: (instance: any) => void;
    options?: MasonryOptions;
    className?: string;
    elementType?: string;
  }

  export default class Masonry extends Component<MasonryPropTypes, void> { }
}

I've also added an interface for the options.

@afram
Copy link
Collaborator

afram commented Jul 9, 2016

Great thanks, this has been merged into v4.2.0

@afram afram closed this as completed Jul 9, 2016
@empz
Copy link
Author

empz commented Jul 10, 2016

@afram Could you add a line to the interface MasonryPropTypes so we can add styles to it?

interface MasonryPropTypes {
    disableImagesLoaded?: boolean;
    updateOnEachImageLoad?: boolean;
    onImagesLoaded?: (instance: any) => void;
    options?: MasonryOptions;
    className?: string;
    elementType?: string;
    style?: Object: // ADDED
  }

@afram
Copy link
Collaborator

afram commented Jul 14, 2016

done v4.2.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants