Skip to content
This repository has been archived by the owner on Nov 13, 2023. It is now read-only.

genType.import of nested types causes name conflicts in TypeScript 3.7.2 #302

Closed
wokalski opened this issue Nov 8, 2019 · 21 comments
Closed

Comments

@wokalski
Copy link
Contributor

wokalski commented Nov 8, 2019

[@genType.import ("./MyMath", "num")]
type num;

Generates:

import {num as num} from './MyMath';

// tslint:disable-next-line:interface-over-type-literal
export type num = num;

When you compile it with tsc you get:

Import declaration conflicts with local declaration of 't'.
@cristianoc
Copy link
Collaborator

@wokalski why regression vs just an issue?

If you rename t in type t = Js.Dict.t(tsunknown); you don't get issues right? Making sure that's the issue.

@cristianoc
Copy link
Collaborator

@wokalski

Btw notice:

[@genType.import "something"]
type t = some_type_definition;

is not a useful pattern.
If the type is imported, then it's not checked against the local definition.

@cristianoc
Copy link
Collaborator

cristianoc commented Nov 8, 2019

I guess what we're looking at there is a feature request, where the above pattern is checked: there's the JS type definition, there's the Reason type definition, and one would like to check that they match.

And I guess match means something like: if the reason type rt maps to js type toJS(rt), then any value of type jstype defined in the JS file is also of type toJS(rt).

@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

Right @cristianoc. I figured it's better to use external and %identity in this case.

@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

I'm closing this one. Seems like an interesting feature @cristianoc but it feels like an error is a desirable result now. Maybe it should even fail during genType execution!

@wokalski wokalski closed this as completed Nov 8, 2019
@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

Reopening:

[@genType.import ("./StyleSheet", "AnyStyleProp")]
type t;

compiles to:

/* TypeScript file generated by genType. */
/* eslint-disable import/first */


import {AnyStyleProp as t} from './StyleSheet';

// tslint:disable-next-line:interface-over-type-literal
export type t = t;

which causes the same error.

@wokalski wokalski reopened this Nov 8, 2019
@cristianoc
Copy link
Collaborator

@wokalski thanks it appears I have zero examples of that pattern, and needs to be fixed.
Thanks for spotting it!

@cristianoc
Copy link
Collaborator

cristianoc commented Nov 8, 2019

@wokalski I have added an example here: #303.

I think that's what you want.
While the tuple argument of @genType.import is used to access classes inside the module.

I strongly suspect this is not exactly clear from the documentation.

@cristianoc
Copy link
Collaborator

Maybe in the case of interfaces one does need that pattern?

@cristianoc
Copy link
Collaborator

Here's a very simple problematic example:

[@genType.import ("./MyMath", "num")]
type num;

gives

import {num as num} from './MyMath';

// tslint:disable-next-line:interface-over-type-literal
export type num = num;

@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

Yes @cristianoc, exactly. Let me put this example in the issue description so that it's clear in the future what it's about.

@wokalski wokalski changed the title Possible regression: genType.import causes name conflicts genType.import of nested types causes name conflicts Nov 8, 2019
@cristianoc
Copy link
Collaborator

@wokalski I have added a few more examples here: #303.
They all type check for me. Including the "problematic" one above.

@cristianoc
Copy link
Collaborator

Maybe it's about choosing different example, or maybe it's about TS version and/or config.

@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

hmm, it's probably the version! I just upgraded 🤦‍♀
Version 3.7.2

@wokalski wokalski changed the title genType.import of nested types causes name conflicts genType.import of nested types causes name conflicts in TypeScript 3.7.2 Nov 8, 2019
@cristianoc
Copy link
Collaborator

@wokalski how did it go after version upgrade?

@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

I had to downgrade and I think it builds correctly now.

@cristianoc
Copy link
Collaborator

Closing optimistically.

@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

I mean that the code generated by genType is incompatible with new versions of typescript! I think this requires a re-open 😄

@cristianoc cristianoc reopened this Nov 8, 2019
@cristianoc
Copy link
Collaborator

I can confirm repro on TS 3.7.2.

@cristianoc
Copy link
Collaborator

@wokalski fixed here: #304

@wokalski
Copy link
Contributor Author

wokalski commented Nov 8, 2019

Great, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants