gulp build step emits declaration files with errors.

[Ionaru]

Summary
A number of declaration files in the scryfall-sdk package contain errors, resulting in build errors on my end.

Details
When using scryfall-sdk in my typescript project, I noticed that errors would pop up when building:

node_modules/scryfall-sdk/out/api/Cards.d.ts:132:5 - error TS2411: Property 'tcgplayer' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

132     tcgplayer?: string | null;
        ~~~~~~~~~

node_modules/scryfall-sdk/out/api/Cards.d.ts:133:5 - error TS2411: Property 'cardmarket' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

133     cardmarket?: string | null;
        ~~~~~~~~~~

node_modules/scryfall-sdk/out/api/Cards.d.ts:134:5 - error TS2411: Property 'cardhoarder' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

134     cardhoarder?: string | null;
        ~~~~~~~~~~~

node_modules/scryfall-sdk/out/api/Cards.d.ts:138:5 - error TS2411: Property 'gatherer' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

138     gatherer?: string | null;
        ~~~~~~~~

node_modules/scryfall-sdk/out/api/Cards.d.ts:139:5 - error TS2411: Property 'tcgplayer_decks' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

139     tcgplayer_decks?: string | null;
        ~~~~~~~~~~~~~~~

node_modules/scryfall-sdk/out/api/Cards.d.ts:140:5 - error TS2411: Property 'edhrec' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

140     edhrec?: string | null;
        ~~~~~~

node_modules/scryfall-sdk/out/api/Cards.d.ts:141:5 - error TS2411: Property 'mtgtop8' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

141     mtgtop8?: string | null;
        ~~~~~~~

Found 7 errors.

I checked out this repo and looked at the files and the way they are created. Running gulp build finished without error, but the Cards.d.ts file it created does contain the errors I noticed in my build.

It looks like your 'gulp build' step is somehow not giving you the feedback that the regular typescript compiler does.
With a small change to your tsconfig:

{
    "compilerOptions": {
        "target": "es2015",
        "module": "commonjs",
        "moduleResolution": "node",
        "forceConsistentCasingInFileNames": true,
        "removeComments": true,
        "declaration": true,
        "strict": true,
        "strictPropertyInitialization": false,
        "noImplicitReturns": true,
        "noUnusedLocals": true,
        "noUnusedParameters": false,
        "noEmitOnError": true,
        "outDir": "./out"
    },
    "files": [
        "src/Scry.ts"
    ]
}

Specifically the noEmitOnError, outDir and files fields, I was able to run the typescript compiler directly, without gulp.
Output of npx tsc:

src/api/Cards.ts:151:2 - error TS2411: Property 'tcgplayer' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

151  tcgplayer?: string | null;
     ~~~~~~~~~

src/api/Cards.ts:152:2 - error TS2411: Property 'cardmarket' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

152  cardmarket?: string | null;
     ~~~~~~~~~~

src/api/Cards.ts:153:2 - error TS2411: Property 'cardhoarder' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

153  cardhoarder?: string | null;
     ~~~~~~~~~~~

src/api/Cards.ts:158:2 - error TS2411: Property 'gatherer' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

158  gatherer?: string | null;
     ~~~~~~~~

src/api/Cards.ts:159:2 - error TS2411: Property 'tcgplayer_decks' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

159  tcgplayer_decks?: string | null;
     ~~~~~~~~~~~~~~~

src/api/Cards.ts:160:2 - error TS2411: Property 'edhrec' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

160  edhrec?: string | null;
     ~~~~~~

src/api/Cards.ts:161:2 - error TS2411: Property 'mtgtop8' of type 'string | null | undefined' is not assignable to string index type 'string | null'.

161  mtgtop8?: string | null;
     ~~~~~~~

src/api/Cards.ts:381:63 - error TS2322: Type 'string | undefined' is not assignable to type 'string | number'.
  Type 'undefined' is not assignable to type 'string | number'.

381     return this.query<Card>(["cards", setCode, collectorNumber, lang]);
                                                                    ~~~~

src/api/Cards.ts:438:36 - error TS2488: Type 'CardIdentifier[] | undefined' must have a '[Symbol.iterator]()' method that returns an iterator.

438             emitter.emitAll("not_found", ...not_found);
                                                ~~~~~~~~~

src/util/MagicQuerier.ts:62:33 - error TS2345: Argument of type 'SearchError | undefined' is not assignable to parameter of type 'SearchError'.
  Type 'undefined' is not assignable to type 'SearchError'.

62              if (result || !this.canRetry(lastError)) break;
                                             ~~~~~~~~~

Found 10 errors.

The first 7 errors are the same as the ones I got in my build, which was to be expected.
The last 3 errors are very surprising to me, they seem to indicate actual problems with the build and I don't know why gulp has let those slip through.

Possible solutions on your end

• Change the settings in gulp to make the errors visible and the build to fail. My knowledge of gulp is limited so I don't know a way to do this off the top of my head.
• Don't rely on gulp to build your typescript files. With the tsconfig changes I mentioned earlier you can use the typescript compiler directly, which will display the errors. There are several alternatives for running scripts automatically (including watch) that do not require gulp, like npm-watch or typescript watch.

Workarounds on my end

• Add skipLibCheck to the tsconfig.
  • Undesirable because this will disable all type checking against library declaration files.

Further notes
I would be happy to help develop a solution for this, but changing of replacing the build system is not something I am comfortable doing without approval of the maintainers.

Possibly related to (or regression of) #11 ?

[ChiriVulpes]

Thanks for the very detailed report! Tbh I'm not sure why I set this up to use gulp-typescript in the first place, most of my newer projects use gulp but use the TS compiler directly, or, for ones that are this simple, they have npm scripts that just use tsc. I'll switch to the latter for this project.

As for the rest of this, I'll investigate all of this in further detail later tonight or tomorrow. Should hopefully have this all resolved then 😅

[ChiriVulpes]

It looks like your errors are because my tsconfig.json didn't have strict on (because I was dumb when I set this up, I guess) so I fixed that, among other updates:

• Now using tsc directly (but still thru gulp so that I can more easily get test running after building)
• Now using eslint
• Fixed a bunch of tests that just... did not work correctly at all??? (how did they ever pass, seriously??)

Should have the fixes released shortly, just writing up a changelog now ✨

[Ionaru]

I can confirm the new version has fixed the build problems I was experiencing! Thank you for the blazingly fast reply and fix! 🚀