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

TS bugfix, export top-level indexer #19

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 [email protected]

@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants