TS bugfix, export top-level indexer #19

Open

karikera wants to merge 1 commits from karikera/patch-1 into master

pull from: karikera/patch-1

merge into: sheetjs:master

sheetjs:master

sheetjs:gh-pages

Conversation 5 Commits 1 Files Changed 1 +30 -26

karikera commented 2020-03-05 09:48:21 +00:00 (Migrated from github.com)

Copy Link

TS bugfix, export top-level indexer

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

TS bugfix, export top-level indexer `export default cptable` does not equal with `module.exports = cptable`, it will export like `module.exports["default"] = cptable`

coveralls commented 2020-03-05 09:52:53 +00:00 (Migrated from github.com)

Copy Link

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

[](https://coveralls.io/builds/29146968) Coverage remained the same at 97.494% when pulling **3d6bf311b8e93561f220fc30d09b75b7888f967d on karikera:patch-1** into **62fd61e2b7122819fb9a49dc83917d3daee881d9 on SheetJS:master**.

SheetJSDev commented 2020-03-05 17:48:37 +00:00 (Migrated from github.com)

Copy Link

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

One of the comments in the original code was: ```typescript /* 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 commented 2020-03-05 18:10:23 +00:00 (Migrated from github.com)

Copy Link

@SheetJSDev

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

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

SheetJSDev commented 2020-03-05 18:18:55 +00:00 (Migrated from github.com)

Copy Link

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?

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: ```typescript import cptable from 'codepage'; ``` This was fixed in the type definition using ```typescript export default cptable; ``` Is there a reason to prefer `export = cptable` over `export default`?

karikera commented 2020-03-05 18:21:43 +00:00 (Migrated from github.com)

Copy Link

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

this module has errors on TS

@SheetJSDev `export default` is not compiled to `module.exports = cptable` `export default cptable` is `module.exports['default'] = cptable` this module has errors on TS 

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin karikera/patch-1:karikera/patch-1

git checkout karikera/patch-1

Merge

Merge the changes and update on Gitea.

git checkout master

git merge --no-ff karikera/patch-1

git checkout master

git merge --ff-only karikera/patch-1

git checkout karikera/patch-1

git rebase master

git checkout master

git merge --no-ff karikera/patch-1

git checkout master

git merge --squash karikera/patch-1

git checkout master

git merge --ff-only karikera/patch-1

git checkout master

git merge karikera/patch-1

git push origin master

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

  

duplicate

  

enhancement

  

invalid

  

question

  

wontfix

No Label

bug

duplicate

enhancement

invalid

question

wontfix

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

1 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sheetjs/js-codepage#19

No description provided.