FIX types in api package

[dgreuel]

Issue: #7069

After installing the contexts addon to my TypeScript project, I was unable to run Storybook due to the following errors:

ERROR in [at-loader] ./node_modules/@storybook/components/dist/syntaxhighlighter/syntaxhighlighter.d.ts:3:36 
    TS7016: Could not find a declaration file for module 'react-syntax-highlighter/prism-light'. 'C:/Projects/Git/my-project/node_modules/react-syntax-highlighter/prism-light.js' implicitly has an
 'any' type.
  Try `npm install @types/react-syntax-highlighter` if it exists or add a new declaration (.d.ts) file containing `declare module 'react-syntax-highlighter/prism-light';`

ERROR in [at-loader] ./node_modules/@storybook/components/dist/syntaxhighlighter/syntaxhighlighter.d.ts:1:23 
    TS2688: Cannot find type definition file for 'react-syntax-highlighter'.

ERROR in [at-loader] ./node_modules/@storybook/router/dist/router.d.ts:2:55 
    TS7016: Could not find a declaration file for module '@reach/router'. 'C:/Projects/Git/my-project/node_modules/@reach/router/index.js' implicitly has an 'any' type.
  Try `npm install @types/reach__router` if it exists or add a new declaration (.d.ts) file containing `declare module '@reach/router';`

ERROR in [at-loader] ./node_modules/@storybook/api/dist/modules/url.d.ts:29:26 
    TS7016: Could not find a declaration file for module '@reach/router'. 'C:/Projects/Git/my-project/node_modules/@reach/router/index.js' implicitly has an 'any' type.
  Try `npm install @types/reach__router` if it exists or add a new declaration (.d.ts) file containing `declare module '@reach/router';`

ERROR in [at-loader] ./node_modules/@storybook/api/dist/modules/versions.d.ts:12:9 
    TS2411: Property 'latest' of type 'Version | undefined' is not assignable to string index type '{ [key: string]: any; }'.

ERROR in [at-loader] ./node_modules/@storybook/api/dist/modules/versions.d.ts:13:9 
    TS2411: Property 'next' of type 'Version | undefined' is not assignable to string index type '{ [key: string]: any; }'.

ERROR in [at-loader] ./node_modules/@storybook/api/dist/modules/versions.d.ts:14:9 
    TS2411: Property 'current' of type 'Version | undefined' is not assignable to string index type '{ [key: string]: any; }'.

ERROR in [at-loader] ./node_modules/@storybook/api/dist/index.d.ts:45:26 
    TS7016: Could not find a declaration file for module '@reach/router'. 'C:/Projects/Git/my-project/node_modules/@reach/router/index.js' implicitly has an 'any' type.
  Try `npm install @types/reach__router` if it exists or add a new declaration (.d.ts) file containing `declare module '@reach/router';`

ERROR in [at-loader] ./node_modules/@storybook/api/dist/index.d.ts:61:13 
    TS2411: Property 'latest' of type 'Version | undefined' is not assignable to string index type '{ [key: string]: any; }'.

ERROR in [at-loader] ./node_modules/@storybook/api/dist/index.d.ts:62:13 
    TS2411: Property 'next' of type 'Version | undefined' is not assignable to string index type '{ [key: string]: any; }'.

ERROR in [at-loader] ./node_modules/@storybook/api/dist/index.d.ts:63:13 
    TS2411: Property 'current' of type 'Version | undefined' is not assignable to string index type '{ [key: string]: any; }'.

ERROR in [at-loader] ./node_modules/@storybook/api/dist/index.d.ts:68:27 
    TS7016: Could not find a declaration file for module '@reach/router'. 'C:/Projects/Git/my-project/node_modules/@reach/router/index.js' implicitly has an 'any' type.
  Try `npm install @types/reach__router` if it exists or add a new declaration (.d.ts) file containing `declare module '@reach/router';`

What I did

I'm not sure why the @reach/router and react-syntax-highlighter types were missing, but I got rid of those errors by adding them to my own devDependencies:

    "@types/reach__router": "^1.2.4",
    "@types/react-syntax-highlighter": "^10.2.1"

I got rid of the TS2411 errors with the changes made in this PR.

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fork-dgreuel-api-versions-fix-types.storybook.now.sh

[shilman]

@ndelangen Is this the correct fix? Or is it better to modify the calling code to accept null/undefined values for these fields? (I suspect it IS better since AFAIK those values can be null!)

[ndelangen]

I'm not sure right now, but I think those values can be undefined indeed.

[dgreuel]

Yeah, it looks like my quick and dirty fix broke the build since those values can be undefined in other places. (I’m not too familiar with the codebase...)

[adamdoyle]

What about something like this:

// extract two interfaces:
export interface UnknownEntries { // (1)
    [key: string]: {
        [key: string]: any;
    };
}
export interface Versions { // (2)
    latest?: Version;
    next?: Version;
    current?: Version;
}

// replace with intersection type:
export interface SubState {
    versions: Versions&UnknownEntries; // <-- previously (1) and (2)
    lastVersionCheck: number;
    dismissedVersionNotification: undefined | string;
}

[rosskevin]

The @types will need to be dependencies (not devDependencies) so that downstream typescript users will install the types packages that are required transitively.

[ndelangen]

@rosskevin do you recon that's the only thing we need to do to resolve the issue?

Which dependencies would need to change from devdependencies to dependencies?

This will add some bloat to the node_modules of non-typescript users :(

[rosskevin]

@ndelangen I suspect @adamdoyle's code above plus @types/reach__router": "^1.2.4" needs to go in dependencies. I commented the indexer locally this allowed me to continue. I'm unaware as to why the types in this definition are open with an indexer, but certainly naming it as suggested by @adamdoyle is a good idea so that types can become more strict later.

I do not consider the types package bloat - I would really like to change that perception. It is a necessary part of the runtime (start-storybook) for typescript users and storybook itself is exporting the same .d.ts files. If adding the needed transitive @types dependencies for ts users is 'bloat', then axiomatically storybook has built-in bloat with the included .d.ts files. I'm not sure the argument holds, but certainly the pros/cons are in favor of including the .d.ts files fully by the mechanisms necessary to prevent userland pain.

[ndelangen]

@rosskevin consider me convinced

[zachlebar]

Any idea of an ETA on this bug fix? Judging by the comments it's close, but there are build issues?