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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colocate types with their resources #11565

Open
RobbieMcKinstry opened this issue Dec 6, 2022 · 0 comments
Open

Colocate types with their resources #11565

RobbieMcKinstry opened this issue Dec 6, 2022 · 0 comments
Labels
area/codegen SDK-gen, program-gen, convert kind/engineering Work that is not visible to an external user language/javascript size/M Estimated effort to complete (up to 5 days).

Comments

@RobbieMcKinstry
Copy link
Contributor

RobbieMcKinstry commented Dec 6, 2022

Hello!

  • Vote on this issue by adding a 馃憤 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

It's been suggested that the current file layout, where types/{{module}}/index.ts declares module types, is less effective than colocating the types with the resources. I'm going to do my best to argue this position, but I confess I don't have a few deep understanding of the API surface of generated types, so please offer criticism and feedback, and correct me where I'm wrong. It's likely I'm missing something that necessitated this file layout in the first place.

  1. It's less idiomatic. There's no real technical benefit to have modules declared at the top level, and module types declared under a separate folder. It makes more sense to have a resource Foo, and all of the types FooArgs, FooFilterRuleArgs`, etc. all located in the same folder. This will minimize the diff in the import path between these resources, cleaning up import paths.

  2. This allows large packages (i.e. Azure Native, AWS-Native) to split along module boundaries. For example, we may be able to take a subset of modules (e.g. s3, ec2, rds) and package them in their own NPM package, cleaving currently massive packages into smaller packages. This will help us maneuver around NPM package size limits. (Corresponding Go issue, Python issue).

Goal: To make this change in a backward-compatible way, we will still need to generates a types directory and re-export the files from the resource modules.

AFAIK this issue is not as simple as dropping the files from #10831) both because we need to be backward-compatible, and because there are conflicting names within input.ts and output.ts for a given module. I'd need to iterate on the new API before starting work on this.

It might be as simple as moving those files, adding the compatibility files in /types, and adding re-exports to the index.ts of each module to namespace the input.ts and output.ts files. For example:

types/ec2/input.ts:BucketCorsConfigurationArgs => ec2/input.ts:BucketCorsConfigurationArgs

Affected area/feature

codegen/nodejs

@RobbieMcKinstry RobbieMcKinstry added language/javascript area/codegen SDK-gen, program-gen, convert kind/engineering Work that is not visible to an external user size/M Estimated effort to complete (up to 5 days). labels Dec 6, 2022
@RobbieMcKinstry RobbieMcKinstry added this to the 0.83 milestone Dec 6, 2022
@mikhailshilkov mikhailshilkov removed this from the 0.83 milestone Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen SDK-gen, program-gen, convert kind/engineering Work that is not visible to an external user language/javascript size/M Estimated effort to complete (up to 5 days).
Projects
None yet
Development

No branches or pull requests

2 participants