Skip to content

TS bugfix, export top-level indexer #19

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants