Extraneous . in generated TS code

[gaku-sei]

GenType: 3.1.0
BuckleScript: 6.2.2-flambda-uniform-array.1 and 6.2.1

When binding some components, an extraneous . will be inserted between the function keyword and the parens () in TypeScript:

[@genType.import "./"] [@react.component]
external make:
  (
    ~className: string=?,
    ~children: React.element=?,
    ~label: string=?,
    ~_to: string,
    ~method: [@bs.string] [ | `push | `replace]=?,
    ~style: ReactDOMRe.Style.t=?
  ) =>
  React.element =
  "default";

Will generate the following code:

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


import {default as defaultNotChecked} from './';

import * as React from 'react';

const $$toJS892729597: { [key: string]: any } = {"-899608102": "push", "724060212": "replace"};

// In case of type error, check the type of 'default' in 'Link.re' and './'.
export const defaultTypeChecked: React.ComponentType<{
  readonly className?: string; 
  readonly children?: JSX.Element; 
  readonly label?: string; 
  readonly to: string; 
  readonly method?: 
  | "push"
  | "replace"; 
  readonly style?: ReactDOMRe_Style_t
}> = defaultNotChecked;

// Export '$$default' early to allow circular import from the '.bs.js' file.
export const $$default: unknown = function .(Arg1: any) { // <- extra `.` here
  const $props = {className:Arg1.className, children:Arg1.children, label:Arg1.label, to:Arg1.to, method:(Arg1.method == null ? Arg1.method : $$toJS892729597[Arg1.method]), style:Arg1.style};
  const result = React.createElement(defaultTypeChecked, $props);
  return result
} as React.ComponentType<{
  readonly className?: string; 
  readonly children?: JSX.Element; 
  readonly label?: string; 
  readonly to: string; 
  readonly method?: 
  | "push"
  | "replace"; 
  readonly style?: ReactDOMRe_Style_t
}>;

import {Style_t as ReactDOMRe_Style_t} from '../../../../app/shims/ReactDOMRe.shim';

export default $$default;

The relevant line code:

export const $$default: unknown = function .(Arg1: any) { // <- extra `.` here

Not all the components generate the ill-formed code above though, and it seems to be related to the use of the [@bs.string] attribute 😕

[cristianoc]

@gaku-sei what's the intent with @genType.import "./"]?
Normally one would write the path to a file inside "...".

[cristianoc]

E.g. see https://github.com/cristianoc/genType#export-and-import-values.

[gaku-sei]

@cristianoc Sorry, I'm a bit confused. Here is the first example using @genType.import:

[@genType.import "./MyMath"] /* JS module to import from. */
/* Name and type of the JS value to import. */
external realValue: complexNumber => float = "";

Where "./MyMath" seems to be the imported file ./MyMath[.bs].js, and "" will be inferred as realValue by BuckleScript, isn't it?

In my example "./" will be resolved as ./index which stands for ./index[.bs].js, and "default" is the imported value. This pattern works perfectly for all of our other components.

Also, writing [@genType.import "./index"] instead of [@genType.import "./index"] fixes the issue, but it's quite misleading, since, again, it works with most of other components, and it's a pretty widespread convention to drop index from the imported paths in Node.js.

[cristianoc]

@gaku-sei can you explain the convention a bit more?
So ./ is the same as ./index but ./MyMath is just taken literally.
How about ./MyMath/? Is it short for ./MyMath/index?

[cristianoc]

@gaku-sei so I think the only corner case is ./ as in case of ./foo/ then foo is a good name for the conversion function.
See the fix here: #299.

Let me know if it looks good.

[cristianoc]

Closing optimistically.

[gaku-sei]

Normally when requiring ./ Node will look for ./index.js (and throw an exception is not present).
When you require ./MyMath Node will first look for ./MyMath.js, and if not present, look for ./MyMath/index.js.
Requiring ./MyMath/ will indeed make Node look for ./MyMath/index.js (and ignore ./MyMath.js).
Trying to require ./MyMath when there are two files ./MyMath/index.js and ./MyMath.js, will make Node requires the later.

Notice that the above rules also work for ../ (which could be an other edge case).

[gaku-sei]

Sorry, I also forgot about . and .. which are also supposed to work (just like ./ and ../)

[gaku-sei]

Just in case, I made a super small repo demonstrating how the above pattern work: https://github.com/gaku-sei/node-resolution

[cristianoc]

@gaku-sei thanks. Added .. support in master.