fix: check dependencies when adding imports #1239
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kuhe
commented
Apr 11, 2024
| @@ -114,19 +142,54 @@ public TypeScriptWriter addIgnoredDefaultImport(String name, String from, String | |||
| */ | |||
| @Deprecated | |||
| public TypeScriptWriter addImport(String name, String as, String from) { | |||
| boolean isNodePackage = from.startsWith("node:"); | |||
There was a problem hiding this comment.
should a class wrap the String input from to provide answers like boolean isNamespaced, boolean isNodeJSNativePackage, String getPackageName etc.?
There was a problem hiding this comment.
That probably should go in Dependency in the long run.
syall
reviewed
Apr 11, 2024
Comment on lines
189
to
+152
| public TypeScriptWriter addImport(String name, String as, PackageContainer from) { | ||
| if (from instanceof Dependency) { | ||
| addDependency((Dependency) from); | ||
| } |
There was a problem hiding this comment.
In the long run, we should move it to taking Dependency as the third parameter.
syall
approved these changes
Apr 11, 2024
trivikr
approved these changes
Apr 11, 2024
syall
approved these changes
Apr 11, 2024
...t-codegen/src/main/java/software/amazon/smithy/typescript/codegen/validation/ImportFrom.java
Show resolved
Hide resolved
syall
approved these changes
Apr 11, 2024
2 tasks
kuhe
added a commit
to kuhe/smithy-typescript
that referenced
this pull request
May 28, 2024
* fix: check dependencies when adding imports * avoid method overload with super type * handle node: prefix package imports * exempt node: prefix packages from enforced registration * remove addImportUnchecked * remove stray deprecation tag * convert import logic to class * comment grammar * add internalapi annotation to ImportFrom
kuhe
added a commit
to kuhe/smithy-typescript
that referenced
this pull request
Jun 28, 2024
* fix: check dependencies when adding imports * avoid method overload with super type * handle node: prefix package imports * exempt node: prefix packages from enforced registration * remove addImportUnchecked * remove stray deprecation tag * convert import logic to class * comment grammar * add internalapi annotation to ImportFrom
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
when calling
addImport, the TSWriter will either add the import or throw an error if it determines the import is a non-native package and has not been added via addDependency.this is to prevent calls to
addImportthat forget to also calladdDependency.