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

handler return type inferred as 'any' #30

Open
universalhandle opened this issue Mar 4, 2021 · 4 comments
Open

handler return type inferred as 'any' #30

universalhandle opened this issue Mar 4, 2021 · 4 comments

Comments

@universalhandle
Copy link
Contributor

universalhandle commented Mar 4, 2021

This might be a regression from one of the two minor releases this week. I'm pretty sure this was working before I upgraded. See update in follow-up comment.

Today I noticed my linter complaining about:

const app: Application
Unsafe assignment of an any value.eslint(@typescript-eslint/no-unsafe-assignment)

...for code like:

import handler from '@curveball/aws-lambda';
// ...
export const router = handler(app);

I believe this is due at least in part to node_modules/@curveball/aws-lambda/dist/index.d.ts exporting an interface directly from @types/aws-lambda. The file contents are:

import { Application } from '@curveball/core';
import { APIGatewayProxyHandler } from 'aws-lambda';
export default function lambdaHandler(app: Application): APIGatewayProxyHandler;

My IDE shows an error for aws-lambda on line 2: Cannot find module 'aws-lambda' or its corresponding type declarations.

Per the manual, a package that exports another package's types should declare the latter as a dependency in package.json rather than devDependency.

I'm not sure if I'm failing to build the dist correctly or what, but making this change alone does not seem to correct the problem.

@universalhandle
Copy link
Contributor Author

universalhandle commented Mar 4, 2021

I don't think this is a regression as I previously suggested. I upgraded this package at the same time I removed @types/aws-lambda from my own project's dependencies.

I must be hacking my project's node_modules directory (or building the distribution) wrong in my attempts to validate the fix I'm proposing.

A workaround for this problem is to install @types/aws-lambda in my own project. This jibes with the warning in the manual.

@evert
Copy link
Member

evert commented Mar 4, 2021

We could add @types/aws-lambda as a peerDependency. Installing this package is indeed mentioned in the manual. I don't like having types packages as 'real' dependencies, because it means that they are also non-dev on whoever is using @curveball/aws-lambda.

You typically want a small production build when deploying on lambda. Maybe even to keep it under the size where AWS still lets you use the code editor. So the fewer (non-dev) dependencies the better.

@universalhandle
Copy link
Contributor Author

I don't like having types packages as 'real' dependencies, because it means that they are also non-dev on whoever is using @curveball/aws-lambda.

That's legit, especially for folks who aren't using TypeScript at all.

We could add @types/aws-lambda as a peerDependency.

I think that might be stretching the purpose of peerDependencies.

Installing this package is indeed mentioned in the manual.

I read about 20 READMEs that day and installed and removed a bunch of packages trying different things out. It's clearly documented -- totally my mistake.

@universalhandle
Copy link
Contributor Author

There are more dependency-related settings in package.json than I realized. Maybe optionalDependencies is the best place for it. I suppose a more formal declaration of optional dependencies than a note in the README might prevent others opening the same issue I did. (Or maybe I just need to work on my reading comprehension!)

It does put the onus on downstream devs to npm install --no-optional if they want to keep the package size down. Not sure if this has any implications for your build process.

Reopening for your consideration.

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

No branches or pull requests

2 participants