feathersjs-ecosystem / configuration

[MOVED] A plugin for configuring a Feathers application
https://github.com/feathersjs/feathers
MIT License
28 stars 15 forks source link

Missing TypeScript declaration file #48

Closed corydeppen closed 7 years ago

corydeppen commented 7 years ago

Would you accept a PR or be willing to add a index.d.ts TypeScript declaration file for this lib? Even if the definitions are not complete yet, adding the following line to a declaration file would prevent a TS error "File '/path-to/feathers-configuration/index.d.ts' is not a module.".

declare module 'feathers-configuration';
daffl commented 7 years ago

I don't think I understand the error. Does that mean Typescript errors on every npm module that doesn't have definitions?

corydeppen commented 7 years ago

@daffl The prior error I mentioned seems to occur when TS detects a declaration file but doesn't have any content (I had commented out the module definition). If the TS declaration file isn't present for a module that's being imported, you'd see this:

Could not find a declaration file for module 'feathers-configuration'. '/path-to/node_modules/feathers-configuration/lib/index.js' implicitly has an 'any' type.
  Try `npm install @types/feathers-configuration` if it exists or add a new declaration (.d.ts) file containing `declare module 'feathers-configuration';`
daffl commented 7 years ago

But there is no declaration at all in this repository. Not that we wouldn't accept one but it should be complete (and not having one at all shouldn't cause any errors).

corydeppen commented 7 years ago

But there is no declaration at all in this repository

That's the issue. If TS is configured to use "strict": true or "noImplicitAny": true the error I mentioned (TS7016) will be thrown and prevent the project from compiling. Folks opting to relax the strictness options won't see the error, but that seems to defeat the purpose of a type checker like TypeScript.

jhanschoo commented 7 years ago

The PR I just created should fix the issue for the current version.

ScreamZ commented 7 years ago

Hey, still having

src/app.ts(9,32): error TS2497: Module '"C:/Users/andre/js-dev/reactnative/api/node_modules/feathers-configuration/index"' resolves to a non-modul
e entity and cannot be imported using this construct.

using this import syntax

import * as configuration from "feathers-configuration";

Are you sure of type definition or am I doing it wrong ?

Please also consider adding @types/config to dependencies. This avoid developers to include this in their own project for this warning node_modules/feathers-configuration/index.d.ts(4,23): error TS2688: Cannot find type definition file for 'config'.

Best regards

jhanschoo commented 7 years ago

If you're using the latest versions (>1.0.0-pre.*) it seems like the file was removed in this commit https://github.com/feathersjs/configuration/commit/4cdff7a515aac4b64c697273e9091861180dc691 .

Sorry, but I won't be able to maintain the definitions file since I'm not using FeathersJS in my stack at the moment.

daffl commented 7 years ago

That is why it is a prerelease 😉 Definitions will be maintained in https://github.com/feathersjs-ecosystem/feathers-typescript and published separately going forward.

ScreamZ commented 7 years ago

@daffl I can only suggest you to publish them using the standardized way. See that link.

Feathers is an awesome tool, built with perfect nice pattern that make the code clear. It deserve an awesome definition file for its packages. Keep it that way :)

@types/feathers
@types/feathers-rest

etc...

Or directly in the lib using the types key of package.json.

Best regards

jhanschoo commented 7 years ago

If my impression is right, publishing by DefinitivelyTyped is for community-created definitions (and indeed FeathersJS should also publish to it) while if there is intended official support it should be published in the officially-maintained packages.