Skip to content

Conversation

@karikera
Copy link

@karikera karikera commented Mar 5, 2020

TS bugfix, export top-level indexer

export default cptable does not equal with module.exports = cptable, it will export like module.exports["default"] = cptable

`export default` will export like `module.exports["default"] = cptable`
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.494% when pulling 3d6bf31 on karikera:patch-1 into 62fd61e on SheetJS:master.

@SheetJSDev
Copy link
Contributor

One of the comments in the original code was:

/* note: TS cannot export top-level indexer, hence default workaround */

I assume this was in an older version of typescript. Can you verify the new export works in older versions of TS? I think we need to test back to 2.2

@karikera
Copy link
Author

karikera commented Mar 5, 2020

@SheetJSDev
image

I compiled index.d.ts without error in TS@2.2

@SheetJSDev
Copy link
Contributor

So I had to add "strictFunctionTypes": true to types/tsconfig.json. To test the type definitions, make tslint and it shows errors like

/tmp/js-codepage/types/codepage-test.ts:1:8 ERROR: 1:8 expect TypeScript@next compile error: Module '"/tmp/js-codepage/types/index"' has no default export. 

The offending test code was:

import cptable from 'codepage';

This was fixed in the type definition using

export default cptable;

Is there a reason to prefer export = cptable over export default?

@karikera
Copy link
Author

karikera commented Mar 5, 2020

@SheetJSDev
export default is not compiled to module.exports = cptable
export default cptable is module.exports['default'] = cptable

this module has errors on TS

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants