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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependency tracking system uses declared names for imported types/guards/classes #218

Open
download13 opened this issue Nov 20, 2022 · 3 comments
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed

Comments

@download13
Copy link
Collaborator

download13 commented Nov 20, 2022

So I was working on #166, which took me down a rabbithole with #162, of which one of the test cases revealed a deeper problem that isn't only restricted to class imports, but any situation where imports are renamed to avoid identifier conflicts.

For example:

import { Timestamp } from './timestamp'
import { Timestamp as TS } from 'other-library/timestamp'

export type TimeThing = {
  ts1: Timestamp
  ts2: TS
}

results in a guard that contains duplicate identifiers or references non-existent identifiers.

This isn't restricted to duplicate import names. The same problem occurs any time imported bindings are renamed:

import * as AnyNameWeWant from './name'`
import SomeOtherWildName from './name'
import { Name as Name2 } from './name'

Including any of these lines will break dependency tracking, and therefor guard generation.

I think the solution to this is to have the code generation and dependency tracking use the imported identifier rather than the declared and exported name. I'm currently working on a solution, but opening an issue first to explain the changes.

@download13 download13 added enhancement New feature or request bug Something isn't working labels Nov 20, 2022
@rhys-vdw
Copy link
Owner

Nice one @download13. An oversight on my part I guess! Or maybe something broken along the way. I could have sworn we had test cases for this.

@dls314
Copy link

dls314 commented May 8, 2023

I wonder if I'm seeing this issue @download13.

I had an interface that was using a type from the AWS CDK that I'm generating a guard for, and that type from the CDK is imported in the CDK's idiomatic style like import { aws_sns as sns } from 'aws-cdk-lib'

That ended up generating a type-guard that made an import from deep within node_modules instead of a similar import statement as was used in the interface, which eventually caused a webpack error.

I've ended up hand-editing the generated type-guard for the moment, but I was searching through issues for a better solution.

@rhys-vdw
Copy link
Owner

rhys-vdw commented May 9, 2023

A PR to fix this issue is certainly welcome @dls314. I think the code base is not too terrifying.

@rhys-vdw rhys-vdw added the help wanted Extra attention is needed label May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants