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

Publish to DefinitelyTyped as @types/rmwc #5

Open
kctang opened this issue Jan 9, 2018 · 5 comments
Open

Publish to DefinitelyTyped as @types/rmwc #5

kctang opened this issue Jan 9, 2018 · 5 comments

Comments

@kctang
Copy link
Contributor

kctang commented Jan 9, 2018

Thanks for providing rmwc-typed. Can we publish it to DefinitelyTyped as @types/rmwc?

@ncknt
Copy link
Owner

ncknt commented Jan 9, 2018

We're discussing whether to publish to @types/rmwc or just have the definition files in the project: rmwc/rmwc#40 (comment)

@kctang: Feel free to comment there too!

@SleeplessByte
Copy link

This project as is should not be published to DefinitelyTyped. I suggest to mimic the flow types much closer, so that edits can be made by anyone.

For example /Base/index.d.ts would be something like

declare module 'rmwc/Base' {
  export { simpleTag, SimpleTagProps } from 'rmwc/Base/simpleTag'
  export { withRipple, WithRippleProps } from 'rmwc/Base/withRipple'
  export { withTheme, WithThemeProps } from 'rmwc/Base/withTheme'
  export { withMDC } from 'rmwc/Base/withMDC'
  export { withMDCToggle } from 'rmwc/Base/withMDCToggle'
  export { noop } from 'rmwc/Base/noop'
}

Which is exactly the same exports the actual javascript library has (in flow + js).

@ncknt ncknt closed this as completed Mar 28, 2018
@ncknt ncknt reopened this Mar 28, 2018
@mutsys
Copy link
Contributor

mutsys commented Apr 7, 2018

I agree with @SleeplessByte that it should not be published as is, but for different reasons.

  1. As the typings are current written, they can only be imported into your project from the index.d.ts definition as:
import {
    Toolbar
} from "rmwc";

This has the unfortunate side effect of importing all of the rmwc widgets into your project, not just the ones you selected for use. The import statement should be able to written instead as:

import {
    Toolbar
} from "rmwc/Toolbar";

which is in accordance with the recommendations of the rmwc author, which will result in only the Toolbar widgets getting imported. Adjusting the type definitions to allow for this in no way prevents you from also having a "master" type definition file that imports everything.

  1. The definition for Base should not be a plain .ts file, it should be a type definition file as well, i.e. - Base.d.ts.

@SleeplessByte
Copy link

If you apply what I said you solve both issues you describe; and I completely agree with those two points.

I think James (rmwc) is still looking to compile definitions automagically, which would be great.

@ncknt
Copy link
Owner

ncknt commented Apr 10, 2018

Thank you @mutsys for your PR. It's now been merged.

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

4 participants