fix(go): unused imports emitted for type unions #3664
Conversation
When imported types are solely referenced by type unions, a go import
is emitted, but is never used (type unions end up represented as opaque
`interface{}` type). This causes compilation failures.
Added a test case for this scenario in particular, and adjusted go code
generation to not emit dependency imports for type unions.
These imports may be re-introduced soon, as we are working to add
dynamic type checking around type unions in go (at which point those
imports would no longer be unused).
Fixes #3399
| @@ -2,4 +2,5 @@ import { overriddenConfig } from '../../jest.config.mjs'; | |||
|
|
|||
| export default overriddenConfig({ | |||
| coveragePathIgnorePatterns: ['/node_modules/', '<rootDir>/test'], | |||
| watchPathIgnorePatterns: ['.*\\.tsx?$'], | |||
There was a problem hiding this comment.
Note - this makes jest --watch a lot more pleasant to use, as it'd only re-run tests onces tsc --watch updates the .js files instead of possibly re-running when the .ts file changed (which yields unexpected outcome).
I'm contemplating moving this up to the baseline configuration but need to evaluate impact first.
| // for (const t of this.typeMap.value) { | ||
| // ret.push(...t.dependencies); | ||
| // } |
There was a problem hiding this comment.
This was left here intentionally as it will likely need to be re-introduced when @MrArnoldPalmer adds runtime type checks for type unions (#3641).
| ).resolves.not.toContain( | ||
| 'github.com/aws/jsii/packages/jsii-pacmak/test/targets/fixtures/base', | ||
| ); |
There was a problem hiding this comment.
Contemplated putting a .toMatchInlineSnapshot instead here, as it'd show the file has the expected content, but this is more compact and expresses the intent best... I'm slightly worried about false negative tests here, though... but decided this is okay.
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
|
Merging (with squash)... |
When imported types are solely referenced by type unions, a go import
is emitted, but is never used (type unions end up represented as opaque
interface{}type). This causes compilation failures.Added a test case for this scenario in particular, and adjusted go code
generation to not emit dependency imports for type unions.
These imports may be re-introduced soon, as we are working to add
dynamic type checking around type unions in go (at which point those
imports would no longer be unused).
Fixes #3399
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.