Multiple definition of type name props when migriting to JSXv4

[Minnozz]

I had my own type props = ... (unrelated to React props, just using that name). Migrating to JSXv4 caused the following error:

rescript: [30/31] src/SchemaEditor-LumiStoreEditor.cmj
FAILED: src/SchemaEditor-LumiStoreEditor.cmj

  We've found a bug for you!
  SchemaEditor

  Multiple definition of the type name props.
  Names must be unique in a given structure or signature.

FAILED: cannot make progress due to previous errors.

The error seems valid (I should rename my own type), but:

• We may have to add this to the migration guide (JSXv4 claims the type name props)
• The error has no source location, which also makes clicking it in VSCode display an error

[mununki]

Thank you for reporting. How about this error msg? What do you think?

[mununki]

Worth adding it into the spec doc.

[Minnozz]

Thank you for reporting. How about this error msg? What do you think?

Looks good. What happens when the user's type props is defined below make? Which one gets the error location? Pointing at make would be clearer than pointing at the user's definition.

[mununki]

Thank you for reporting. How about this error msg? What do you think?

Looks good. What happens when the user's type props is defined below make? Which one gets the error location? Pointing at make would be clearer than pointing at the user's definition.

I don’t have a good solution for that, literally it is correct that the duplicated following one should
be pointed.

[mununki]

Is it helpful semething like “… If it is inside of component, the type props could be defined already.”? What do you think?

[Minnozz]

Are multiple locations, or hints, supported? Other languages often say "Previous definition was here:"

[cristianoc]

It's technically possible to annotate the props type definition so that error messages can special case it.
(And help the editor extension too).

As for multiple locations, that's a separate question. How about create another issue.

[mununki]

The comment deleted and relocated to rescript-lang/rescript#5695

[mununki]

rescript-lang/rescript#5695 for multiple locations issue

[cristianoc]

The current PR seems a clear improvement, we can merge it.
Can be improved further separately.