bevry / istextorbinary

Determine if a filename and/or buffer is text or binary. Smarter detection than the other solutions.
Other
148 stars 17 forks source link

TypeScript error when using `"moduleResolution": "node16"` #270

Closed cedx closed 1 year ago

cedx commented 2 years ago

I've recently switched from "moduleResolution": "node" to "moduleResolution": "node16" in a project using this library. And I get TypeScript errors:

node_modules/istextorbinary/edition-es2019-esm/index.js:14:24 - error TS7006: Parameter 'filename' implicitly has an 'any' type.
14 export function isText(filename, buffer) {
                          ~~~~~~~~
node_modules/istextorbinary/edition-es2019-esm/index.js:14:34 - error TS7006: Parameter 'buffer' implicitly has an 'any' type.
14 export function isText(filename, buffer) {
                                    ~~~~~~
node_modules/istextorbinary/edition-es2019-esm/index.js:45:26 - error TS7006: Parameter 'filename' implicitly has an 'any' type.
45 export function isBinary(filename, buffer) {
                            ~~~~~~~~
node_modules/istextorbinary/edition-es2019-esm/index.js:45:36 - error TS7006: Parameter 'buffer' implicitly has an 'any' type.
[...]
Found 16 errors in the same file, starting at: node_modules/istextorbinary/edition-es2019-esm/index.js:14

It seems that when using node16 module resolution, the typings located in the compiled-types folder are not loaded anymore by the TypeScript compiler.

Cf. https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#package-json-exports-imports-and-self-referencing I guess that when we use an export map in a library, we must also add a types property to this export map...

Node.js 18.2.0, TypeScript 4.7.3, istextorbinary 6.0.0.

balupton commented 2 years ago

This change will need to be implemented in https://github.com/bevry/boundation which scaffolds the meta files across all bevry's projects.

In my 15 years of working in the tech industry, I've probably only earned about 150k, so I have no desire to do this asap, as there is no ROI for me.

A PR on boundation is welcome, which I can give publish access to the PR author.

balupton commented 1 year ago

Boundation successfully updated this package and didn't encounter this error. Is it still a problem for you?

cedx commented 1 year ago

Thanks for your efforts. Unfortunately, I no longer use TypeScript (I replaced it with Haxe), so I don't know if this problem is still present.

But I suppose it is still here because I don't see the typings declared in the proper section of the package.json file.

"types": "./compiled-types/",
"exports": {
  "node": {
    "import": "./edition-es2019-esm/index.js",
    "default": "./index.cjs",
    "require": "./edition-es2022/index.js"
  },
  "browser": {
    "import": "./edition-browsers/index.js"
  }
}

types should be declared in the exports section. Something like:

"exports": {
  "node": {
    "types": "./compiled-types/",
    "import": "./edition-es2019-esm/index.js",
    "default": "./index.cjs",
    "require": "./edition-es2022/index.js"
  },
  "browser": {
    "types": "./compiled-types/",
    "import": "./edition-browsers/index.js"
  }
}
balupton commented 1 year ago

Ahh okay, now I understand. I'll roll this out in my current batch of automation work. Thanks for bringing this to my attention.

balupton commented 1 year ago

this should now be resolved with v8.2.0 https://github.com/bevry/istextorbinary/commit/7ceccaa53df3f19b275f833f0be3a67b57c07883#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

shoutout to my GitHub Sponsors for affording me the time to make this happen