Skip to content

Maintenance: simplify and revisit tsconfig files & settings #617

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

Closed
saragerion opened this issue Mar 3, 2022 · 7 comments · Fixed by #1667
Closed

Maintenance: simplify and revisit tsconfig files & settings #617

saragerion opened this issue Mar 3, 2022 · 7 comments · Fixed by #1667
Assignees
Labels
completed This item is complete and has been merged/shipped internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
Milestone

Comments

@saragerion
Copy link
Contributor

Description of the feature request

Right now the tsconfig files located in the folder of each utility (Logger, Metrics, Tracer) have repeated settings with some differences.
Example: resolveJsonModule in the Logger utility, but there might be others.
Would be good to revisit the current tsconfig setup and the way we transpile our code.

Problem statement

Inconsistent, potentially unnecessary tsconfig settings across the same utility and different utilities.

Summary of the feature

Simplify tsconfig files & settings.

Code examples

I like the way the AWS SDK v3 does it, extending one another:
https://github.com/aws/aws-sdk-js-v3/tree/main/clients/client-s3

Benefits for you and the wider AWS community

Simplification and better governance.

Describe alternatives you've considered

N/A

Additional context

This issue affects all existing utilities.

Related issues, RFCs

@saragerion saragerion added good-first-issue Something that is suitable for those who want to start contributing triage This item has not been triaged by a maintainer, please wait labels Mar 3, 2022
@saragerion saragerion added this to the production-ready-release milestone Mar 3, 2022
@goverdhan07
Copy link
Contributor

Hi @saragerion! as far as I have understood, this issue is regarding making one common file for tsconfig for all three folders(Logger, Metrics, Tracer) which has similar settings, and extend individual/different settings in each of the folder through new tsconfig files. ryt? please let me know if I'am missing something here :) Thanks!

@dreamorosi
Copy link
Contributor

@goverdhan07 thanks for commenting the issue, I understand that what you're mentioning is correct but there might also be some settings in the tsconfig.json files that are unnecessary and that might be possible to remove. Ideally we'd need to go through all of them, extend when possible and remove when redundant.

@ghost
Copy link

ghost commented Mar 8, 2022

Also note that there are some additional tsconfig files like tsconfig-dev.json and tsconfig.es.json. If they are not needed, they could be deleted. And another note: tsconfig.json only includes "src/**/*" but not tests, so all the compiler settings (like e.g. "strict": true) won't be applied to the test code which is a bit unfortunate (e.g. no consistent live linting in VSCode for tests).

@saragerion saragerion removed this from the production-ready-release milestone May 16, 2022
@dreamorosi dreamorosi added confirmed The scope is clear, ready for implementation help-wanted We would really appreciate some support from community for this one internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) and removed priority:medium triage This item has not been triaged by a maintainer, please wait labels Nov 13, 2022
@dreamorosi dreamorosi changed the title Build (all): simplify and revisit tsconfig files & settings Maintenance: simplify and revisit tsconfig files & settings Nov 14, 2022
@dreamorosi
Copy link
Contributor

Not sure if related, but in #1373, and specifically in point 4 of the issue it's mentioned that even though strict: true is applied in the config, interfaces are not flagging errors when a class that implements them does so incorrectly, i.e.:

const MetricUnit = {
    Bites: 'Bites'
} as const;
type MetricUnit = typeof MetricUnit;

interface MetricsInterface {
    addMetric(name: string, unit: MetricUnit, value: number): void
}

class Metric implements MetricsInterface {
    // Should show an error saying that `addMetric` is not compatible with the one defined in the interface
    public addMetric(name: string, unit: MetricUnit, value: number, resolution: string): void {
        console.log(name, unit, value);
    }
}

This playground for instance instead works.

@dreamorosi
Copy link
Contributor

Additionally, I have looked into this:

Also note that there are some additional tsconfig files like tsconfig-dev.json and tsconfig.es.json. If they are not needed, they could be deleted. And another note: tsconfig.json only includes "src/**/*" but not tests, so all the compiler settings (like e.g. "strict": true) won't be applied to the test code which is a bit unfortunate (e.g. no consistent live linting in VSCode for tests).

The reason why we have these separate config files that selectively include the tests are that we want both src/**/* and tests/**/* to be part of the config but we want only the content of src to be included in the build output. We could however simplify the setup and override only that setting like suggested in the OP.

@dreamorosi dreamorosi added internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) and removed good-first-issue Something that is suitable for those who want to start contributing help-wanted We would really appreciate some support from community for this one internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) labels Jun 7, 2023
@dreamorosi dreamorosi self-assigned this Aug 28, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@github-actions github-actions bot added pending-release This item has been merged and will be released soon and removed confirmed The scope is clear, ready for implementation labels Sep 4, 2023
@dreamorosi dreamorosi added this to the Version 2.0 milestone Sep 13, 2023
@github-actions
Copy link
Contributor

This is now released under v1.13.0 version!

@github-actions github-actions bot added completed This item is complete and has been merged/shipped and removed pending-release This item has been merged and will be released soon labels Sep 18, 2023
@dreamorosi dreamorosi moved this from Coming soon to Shipped in Powertools for AWS Lambda (TypeScript) Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed This item is complete and has been merged/shipped internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
Projects
Development

Successfully merging a pull request may close this issue.

4 participants