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

Wrong type for select(...) in addon-knobs #7348

Closed
kroeder opened this issue Jul 9, 2019 · 14 comments
Closed

Wrong type for select(...) in addon-knobs #7348

kroeder opened this issue Jul 9, 2019 · 14 comments
Labels
Milestone

Comments

@kroeder
Copy link
Member

kroeder commented Jul 9, 2019

Describe the bug
Prop-Options are typed wrong IMO

export interface SelectTypeOptionsProp {
  [key: string]: SelectTypeKnobValue;
}

knobs now only accept a key/value pair instead of e.g. string[]

My story with

selectionMode: select('selectionMode', ['none', 'multiple', 'single'], 'none', undefined),

now fails with

TS2345: Argument of type 'string[]' is not assignable to parameter of type 'SelectTypeOptionsProp'.

Expected behavior
No breaking change

Suggestion
Either make it unknown, any or add a generic

selectionMode: select<string[]>('selectionMode', ['none', 'multiple', 'single'], 'none', undefined),

System:

  • Framework: [angular]
  • Addons: [addon-knobs]
  • Version: [e.g. 5.2.0-alpha.38]

cc @emilio-martinez

@emilio-martinez
Copy link
Contributor

@kroeder got a PR for you to review

@shilman shilman added this to the 5.2.0 milestone Jul 9, 2019
@shilman
Copy link
Member

shilman commented Jul 10, 2019

Olé!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.0-alpha.39 containing PR #7356 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jul 10, 2019
@Evalon
Copy link

Evalon commented Jul 11, 2019

Still getting wrong type if using with "as const".
For example:
const color = ['red', 'blue'] as const <Button color={select('color', color, 'red')} />
Actual result:
TS2345: Argument of type 'readonly ["red", "blue"]' is not assignable to parameter of type 'SelectTypeOptionsProp'.
Type 'readonly ["red", "blue"]' is not assignable to type 'Record<string, SelectTypeKnobValue>'.
Index signature is missing in type 'readonly ["red", "blue"]'.
Expected:
No errors

If "as const" is removed all works properly.

Versions:
TS: 3.5.3
Storybook: 5.2.0-alpha.40

@emilio-martinez
Copy link
Contributor

@Evalon it looks like Typescript treats ReadonlyArray differently to normal arrays for some reason. I opened a PR that should cover that use case.

@shilman
Copy link
Member

shilman commented Jul 18, 2019

Yay!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.0-beta.1 containing PR #7411 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jul 18, 2019
@Evalon
Copy link

Evalon commented Jul 18, 2019

Now error is shown in /node_modules/@storybook/addon-knobs/dist/components/types/Select.d.ts when trying to build by tsc.
Error:(4, 125) TS2344: Type 'T' does not satisfy the constraint 'string | number | symbol'.
Type 'SelectTypeKnobValue' is not assignable to type 'string | number | symbol'.
Type 'undefined' is not assignable to type 'string | number | symbol'.
Type 'T' is not assignable to type 'symbol'.
Type 'SelectTypeKnobValue' is not assignable to type 'symbol'.
Type 'undefined' is not assignable to type 'symbol'.

TS: 3.5.3 (strict)
Storybook: 5.2.0-beta.1

@Evalon
Copy link

Evalon commented Jul 18, 2019

also node_modules/@storybook/addon-knobs/dist/components/types/Color.d.ts(2,29):
error TS7016: Could not find a declaration file for module 'react-color'.

@emilio-martinez
Copy link
Contributor

I think some of the errors you may be getting are because you're building with the strict flag on—Storybook isn't yet.

I'll look into enabling ts strict compilation in that add-on, but that kind of seems like a separate issue. In the meantime, your read-only array use case is covered.

@Evalon
Copy link

Evalon commented Jul 18, 2019

You right but I think it's kind of regression because with @types/storybook__addon-knobs and storybook 5.2.0-alpha.37 I have proper type and don't get any errors.
Anyway thank you for your work. Maybe I look into this problem later and create PR.

@emilio-martinez
Copy link
Contributor

Yeah, totally get where you're coming from. Appreciate the patience 🙏 It's a bit harder here because the types need to actually match with the code, and unfortunately the @types weren't entirely accurate.

I actually had some time this morning to start taking a look, so I'm already working on enabling strict in this add-on. Hoping I'll have something to submit soon.

@shilman
Copy link
Member

shilman commented Jul 19, 2019

@emilio-martinez are you on the storybook discord? would love to chat for a few mins about this!

@shilman
Copy link
Member

shilman commented Jul 22, 2019

Yippee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.0-beta.6 containing PR #7515 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

@Evalon
Copy link

Evalon commented Sep 4, 2019

In TS 3.6.2 is broken.

Error:(35, 26) TS2345: Argument of type 'readonly ("search" | "plus")[]' is not assignable to parameter of type 'SelectTypeOptionsProp<"plus">'.
Type 'readonly ("search" | "plus")[]' is not assignable to type 'readonly "plus"[]'.
Type '"search" | "plus"' is not assignable to type '"plus"'.
Type '"search"' is not assignable to type '"plus"'.

@shilman shilman reopened this Sep 4, 2019
@Evalon
Copy link

Evalon commented Sep 9, 2019

closed by #8027

@shilman shilman closed this as completed Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants