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

audit eslint / prettier configs - not catching uninitialized class members #116

Closed
cprice404 opened this issue Dec 1, 2022 · 4 comments · Fixed by #196
Closed

audit eslint / prettier configs - not catching uninitialized class members #116

cprice404 opened this issue Dec 1, 2022 · 4 comments · Fixed by #196
Assignees

Comments

@cprice404
Copy link
Contributor

Check and make sure that the eslint/prettier configs are up to date with what we've been using internally.

In particular - I noticed that if I was writing a class that had some private readonly member variables, and those variables were not initialized in the constructor, I didn't get a build/lint error. In our CDK repos I believe we would catch that at compile time.

@cprice404 cprice404 added this to the datastructures - 1.0 milestone Dec 1, 2022
@cprice404
Copy link
Contributor Author

task here would be to compare the eslint and prettier configs in both the example dir and the root dir with the ones from our cdk projects, such as:

https://github.com/momentohq/cacheadmin-v2/tree/main/infrastructure/aws

and

https://www.notion.so/momentohq/All-Things-TypeScript-e350a4ee90874a2e84ab1f4169f4b998#8643773be2794656848a52420f4f1c94

And make sure nothing is terribly out of sync between them. Possibly also test the specific scenario described above and see if I am just misremembering or if there is an eslint setting that catches the case where you have an uninitialized readonly member var.

@schwern schwern self-assigned this Jan 7, 2023
@schwern
Copy link
Contributor

schwern commented Jan 9, 2023

@cprice404 This appears to be a typescript thing governed by noUnusedLocals.

We have turned it off everywhere but momentohq/serverless-api-demo/infra/aws/tsconfig.json.

Turning it on in client-sdk-javascript reveals a couple issues.

src/errors/errors.ts:58:20 - error TS6133: '_stack' is declared but its value is never read.

58   private readonly _stack: string | undefined;
                      ~~~~~~

src/simple-cache-client.ts:49:20 - error TS6133: 'logger' is declared but its value is never read.

49   private readonly logger: Logger;
                      ~~~~~~

@bruuuuuuuce @eaddingtonwhite Do you know why noUnusedLocals is off?

@schwern
Copy link
Contributor

schwern commented Jan 9, 2023

@cprice404
Copy link
Contributor Author

okay. Thanks for looking into it @schwern . I think we can just put this on the backburner for now.

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 a pull request may close this issue.

2 participants