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

Add TypeScript Definition #33

Closed
Gi972 opened this issue Apr 3, 2016 · 25 comments
Closed

Add TypeScript Definition #33

Gi972 opened this issue Apr 3, 2016 · 25 comments

Comments

@Gi972
Copy link

Gi972 commented Apr 3, 2016

Hello,

Good Tool, can you add a TypeScript Definition for use with *.tsx files.

Thanks

Gi

@tomitrescak
Copy link
Contributor

Is this what you are after? @arunoda you can maybe add it? Or do you want a PR?

declare var module: any;

declare module "@kadira/storybook" {
  interface Story {
      add (storyName: string, callback: Function): Story;
  }

  export function storiesOf(name: string, module: any): Story;
  export function action(name: string): void;
}

@tomitrescak
Copy link
Contributor

You also need to extend your definition of webpack.config.js to following. @arunoda, resolving jsx should be supported by default in my opinion.

const path = require('path');
module.exports = {
  module: {
    resolve: {
      extensions: ['', '.js', '.jsx']
    },
    loaders: [
      {
        test: /\.jsx?$/,
        loader: 'babel',
        query: { presets: ['react', 'es2015', 'stage-2'] },
        exclude: [path.resolve('./node_modules'), path.resolve(__dirname, 'node_modules')],
        include: [path.resolve('./'), __dirname],
      },
      {
        test: /\.css?$/,
        loaders: [ 'style', 'raw' ],
        include: path.resolve(__dirname, '../')
      }
    ]
  }
};

@tomitrescak
Copy link
Contributor

Hmm, resolving does not seem to work .. not sure why .. the jsx files are not resolved ;(

@arunoda
Copy link
Member

arunoda commented Apr 3, 2016

@tomitrescak I am not good with TS, yes send me a PR.
Also add .configure() method as a definition.

BTW, how TS and .jsx extentions are related?

@tomitrescak
Copy link
Contributor

tsx are compiled into jsx in some configurations (e.g. mine). I am not using typescript loaders or typescript compiler in Meteor, but am compiling TS files into JS. This is faster and more controllable.

@arunoda
Copy link
Member

arunoda commented Apr 4, 2016

Okay. got it.
I took the jsx PR and now in master.
Will do a release in few hours.

@arunoda
Copy link
Member

arunoda commented Apr 4, 2016

BTW: could you work on the TS definition file. Not sure where to add it.

@Gi972
Copy link
Author

Gi972 commented Apr 4, 2016

Thanks guys!

The best place for typescript definition is on the definitelytyped site.
http://definitelytyped.org/

it's a catalogue for typescript definition.

@tomitrescak
Copy link
Contributor

@Gi972 I submitted a pull request, yet they need to include a test file with it as well. And I have noo idea what to put in those tests. Have you done a PR there before?

I also have a full d.t.s definition for Mantra ready, which I'm using personally and I can share it .. yet again .. no idea what tests should be there.

@tomitrescak
Copy link
Contributor

Another problem is that storybook does not have a npmjs page, therefore it is not recognised by the DT CI workflow.

@arunoda
Copy link
Member

arunoda commented Apr 5, 2016

We have a NPM page.
See: https://www.npmjs.com/package/@kadira/storybook/

@tomitrescak
Copy link
Contributor

Yea, I know, but the silly CI scrip only check for high level npmjs addresses such as https://www.npmjs.com/package/storybook/

@Gi972
Copy link
Author

Gi972 commented Apr 5, 2016

hello Tomitrescak , I never make PR before, I don't know how it works.
For the test , I think you have already tcheck it : http://definitelytyped.org/guides/contributing.html#tests

It's right, I never saw the npm module name like @username/module-npm-Name I think it's not very conventional.
Does you think rename it like it: https://www.npmjs/package/storybook after @arunoda ?
It will better for the future not for only this case.

@arunoda
Copy link
Member

arunoda commented Apr 5, 2016

@Gi972 yes it not. That's something NPM introduce recently. This is very important because of the recent NPM related issues.

If that user have used a scoped package, there may not such an issue.

@camwest
Copy link

camwest commented Apr 28, 2016

Definitely Typed is deprecated in favour of https://github.com/typings/typings. It's much easier to add to.

@tomitrescak
Copy link
Contributor

Great catch! I'll add it as soon as I'll have a bit of time.

@tomitrescak
Copy link
Contributor

@camwest I simply do not have the time. Yet, @arunoda, if you make the typings part of your package (above), it can be easily added to the typings repository.

@arunoda
Copy link
Member

arunoda commented May 3, 2016

@tomitrescak You mean, use typescript?

@tomitrescak
Copy link
Contributor

tomitrescak commented May 3, 2016

No, making the typescript definition as a part of the package and including it in package.json:

// file lib/typings.d.ts
declare var module: any;

declare module "@kadira/storybook" {
  interface Story {
      add (storyName: string, callback: Function): Story;
  }

  export function storiesOf(name: string, module: any): Story;
  export function action(name: string): void;
}

and in package.json

"typings": "./lib/typings.d.ts"

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

Do you want a PR for this?

@arunoda
Copy link
Member

arunoda commented May 3, 2016

@tomitrescak Why not. Send me a PR.

@arunoda
Copy link
Member

arunoda commented May 5, 2016

@tomitrescak This is merged and released with v1.20.0. What else do we need to have to do?

@tomitrescak
Copy link
Contributor

All you need to do is enjoy the smiles of Typescript developers that will love you for this;) and occasionally update the typing when API changes. If you'll need help, just ping me. Thanks!

@tomitrescak
Copy link
Contributor

And you can close this now. Thanks!

@arunoda
Copy link
Member

arunoda commented May 5, 2016

Thanks @tomitrescak.
I think I can update the def file with API changes :)

@arunoda arunoda closed this as completed May 5, 2016
@Gi972
Copy link
Author

Gi972 commented May 6, 2016

@tomitrescak @arunoda
Guys thanks a lot for your time

;-)

ndelangen pushed a commit that referenced this issue Apr 5, 2017
When some url params are already present ui is displayed in the default way
ndelangen pushed a commit that referenced this issue Apr 5, 2017
ndelangen pushed a commit that referenced this issue Apr 11, 2017
ndelangen pushed a commit that referenced this issue Apr 11, 2017
ndelangen added a commit that referenced this issue Apr 11, 2017
thani-sh pushed a commit that referenced this issue May 24, 2017
Run without silent for debugging
ndelangen pushed a commit that referenced this issue Feb 23, 2024
…-story-flow

Implement "How to write your Story" flow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants