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

fix(koa): Fix missing dependency on feathers #3061

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

vonagam
Copy link
Member

@vonagam vonagam commented Feb 16, 2023

@feathersjs/feathers should be in dependencies, not in devDependencies, because the koa package uses feathers types in its own exported types: in Application type and in augmentation of koa's ExtendableContext.

@netlify
Copy link

netlify bot commented Feb 16, 2023

‼️ Deploy request for feathers-dove rejected.

Name Link
🔨 Latest commit 56a8443

@daffl
Copy link
Member

daffl commented Feb 17, 2023

Thank you. I wish there was a way to easily test for this but I don't think there is.

@daffl daffl merged commit 80dc95f into feathersjs:dove Feb 17, 2023
@vonagam vonagam deleted the fix-koa-feathers-not-dev branch February 17, 2023 00:18
@vonagam
Copy link
Member Author

vonagam commented Feb 17, 2023

Yeah, nothing comes to mind unfortunately.

Does not help that npm is the most popular package manager and with node_modules even if a package is not listed as a dependency but actually present in that folder it will be resolved. So for most users it will not be a problem and will not get reported.

@vonagam
Copy link
Member Author

vonagam commented Feb 18, 2023

Hm... actually there should be a way to detect it. After lib folder is built a script can look for require("$x") and from '$x' strings with $x being a name from devDependencies field. That's it.

Just looked and found depcheck. By default it includes devDependencies, so basic cli invocation will not do, but its default export depcheck(rootDir, options, callback) has package field in options. So it is possible to parse package.json then clear devDependencies and pass it to that function. But I don't see an appropriate place where such utility script will be put in feathers repo.

Oh, there is also npm-check which is based on depcheck but does have --production flag to skip devDependencies, so maybe this one is a way to go.

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.

None yet

2 participants