search

smithy-lang / smithy-typescript

Smithy code generators for TypeScript. (in development)
Apache License 2.0
211 stars 78 forks source link

Export types with `export type` #1284

Closed libre-man closed 1 month ago

libre-man commented 1 month ago

Description of changes: Some types were exported with export instead of export type which is causing issues in our build system. This indicates more clearly that a type is exported, and no runtime behaviour is changed.

kuhe commented 1 month ago

This is a problem with the build system, since type export/import statements are optional.

What is the error?

libre-man commented 1 month ago

The problem is that we strongly prefer to consume the source code (better go-to-definition, faster builds for us), not the compiled code. Webpack then complains about it not being to find the things being exported here, as they refer to types.

What would be the downside of having these exported as types instead of normal exports?

kuhe commented 1 month ago

You generate the client source code and merge it into a larger compiled application? That's fine.

I'll let the team know to review this PR and we will likely merge it, but the real fix is going to be in your webpack configuration.

You can also configure a post-generation step. You may have a linting/formatting transform applied after code generation, and in that you can add https://typescript-eslint.io/rules/consistent-type-exports/ or https://biomejs.dev/linter/rules/use-export-type/.

syall commented 1 month ago

@libre-man It looks like the Java checkstyle is failing, can you resolve the errors?

kuhe commented 1 month ago 
IndexGenerator.java:118: Line is longer than 120 characters (found 122). [LineLength]
libre-man commented 1 month ago

@libre-man It looks like the Java checkstyle is failing, can you resolve the errors?

Done @syall!

libre-man commented 1 month ago

Anything I need to do get this merged still?

@kuhe