catamphetamine / read-excel-file

Read *.xlsx files in a browser or Node.js. Parse to JSON with a strict schema.
https://catamphetamine.gitlab.io/read-excel-file/
MIT License
301 stars 52 forks source link

TypeScript problem with native ECMAScript Module support #111

Open peterhirn opened 1 year ago

peterhirn commented 1 year ago

When using TypeScript with native ECMAScript Module support (eg. "moduleResolution": "nodenext") and "type": "module" importing from read-excel-file/node produces the following error:

> tsc --build

node_modules/.pnpm/read-excel-file@5.6.0/node_modules/read-excel-file/node/index.d.ts:13:8 - error TS2307: Cannot find module '../types.d' or its corresponding type declarations.

13 } from '../types.d';
          ~~~~~~~~~~~~

node_modules/.pnpm/read-excel-file@5.6.0/node_modules/read-excel-file/node/index.d.ts:21:8 - error TS2307: Cannot find module '../types.d' or its corresponding type declarations.

21 } from '../types.d';

I patched index.d.ts and node/index.d.ts and this seems to work, even for CJS imports and other settings (eg. "moduleResolution": "node") :

--- a/index.d.ts
+++ b/index.d.ts
@@ -4,7 +4,7 @@ import {
    ParseWithoutSchemaOptions,
    ParsedObjectsResult,
    Row
-} from './types.d';
+} from './types.d.js';

 export {
    Schema,
@@ -13,7 +13,7 @@ export {
    Integer,
    Email,
    URL
-} from './types.d';
+} from './types.d.js';

--- a/node/index.d.ts
+++ b/node/index.d.ts
@@ -10,7 +10,7 @@ import {
    ParseWithoutSchemaOptions,
    ParsedObjectsResult,
    Row
-} from '../types.d';
+} from '../types.d.js';

 export {
    ParsedObjectsResult,
@@ -18,7 +18,7 @@ export {
    Integer,
    Email,
    URL
-} from '../types.d';
+} from '../types.d.js';

Not sure if this is a good patch or if it would break other users code. Feedback from anyone who's more familiar with TypeScript and native ECMAScript Module support would be appreciated.

catamphetamine commented 1 year ago

Hi.

First of all, there're no ./types.d.js files in the repo, only ./types.d.ts ones.

Second, regular TypeScript doesn't allow importing files with .ts extension: https://bobbyhadz.com/blog/typescript-import-path-cannot-end-with-ts-extension

peterhirn commented 1 year ago

Importing files with an additional .js extension is necessary when using TypeScript + ECMAScript Modules in Node.js, see https://www.typescriptlang.org/docs/handbook/esm-node.html

This code works in CommonJS modules, but will fail in ES modules because relative import paths need to use extensions. As a result, it will have to be rewritten to use the extension of the output of foo.ts - so bar.ts will instead have to import from ./foo.js.

ps. my app has about 30 direct dependencies and read-excel-file is the only one which doesn't work with "moduleResolution": "nodenext".

catamphetamine commented 1 year ago

I see. So looks like TypeScript ESM is incompatible with regular TypeScript. It’s weird that they haven’t implemented backwards compatibility though similsr to Node.js’es where they support importing non-type-module modules from type-module modules. Most likely there is backwards compatibility, just under some flag.

On Wed, 8 Feb 2023 at 10:40, peterhirn @.***> wrote:

Importing files with an additional (non-existent) .js extension is necessary when using TypeScript + ECMAScript Modules in Node.js, see https://www.typescriptlang.org/docs/handbook/esm-node.html

This code works in CommonJS modules, but will fail in ES modules because relative import paths need to use extensions. As a result, it will have to be rewritten to use the extension of the output of foo.ts - so bar.ts will instead have to import from ./foo.js.

ps. my app has about 30 direct dependencies and read-excel-file is the only one which doesn't work with "moduleResolution": "nodenext".

— Reply to this email directly, view it on GitHub https://github.com/catamphetamine/read-excel-file/issues/111#issuecomment-1422165883, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADUP35P7EMZQU3IIXWOV5LWWNEXFANCNFSM6AAAAAAUUKRJ4Y . You are receiving this because you commented.Message ID: @.***>

peterhirn commented 1 year ago

I couldn't find any additional compatibility flag. It seems adding .js to typescript imports always works, regardless of moduleResolution config. But moduleResolution: nodenext requires it.