Discussion: Migrate codebase to TypeScript (from Flow)

[bengry]

I hope this is the correct place to put this - I didn't find any existing similar issue, and also the Spectrum chat seems like a worse place to put this up for discussion, since it's less visible than an issue. If this is out of place, feel free to close this, but please redirect me to where this discussion can take place.

---

With a lot of the community moving towards TypeScript over Flow (see Yarn and Jest as big open source examples), is there a place for Popper.js, and specifically popper-react to be written in TypeScript instead of Flow? It would surely make maintaining TypeScript types either (at the expense of Flow of course, which would have to be generated/written separately). I'm focusing on popper-react over the code project since it's a lot smaller (~2K JS LOC for the react project vs ~4.5K JS LOC for the core project), and migration shouldn't be very difficult from having taken a look at some of the codebase.

Packages versions

• Popper.js: N/A
• react-popper: 2.x

[FezVrasta]

Hi, I don’t have any plan to migrate to TypeScript, it would degrade the type safety of the code base.

TypeScript is okay for people that are looking for a way to have autocomplete/intellisense functionalities, but it’s not suitable as a type system.

[bengry]

@FezVrasta what do you mean? I don't have much experience with Flow, but TypeScript gives you lot much than just autocomplete. If you set it to strict mode (which you should when using it, unless there are good reasons not to), you get a very strong type system.

Do you have an example for something that wouldn't be possible in TS that's used in the repo, that Flow covers?

[FezVrasta]

TypeScript has basically no type-inference

TS:

Flow:

TS:

Flow:

Also, I don't like the way the TypeScript maintainers forced themselves as "the only JavaScript type system" by reserving for themselves the package.json#types property,

[bengry]

Regarding your first example, I agree TS couldn't probably inferred it in this case, but in sake of consistency, it does not (again, just my thoughts), since the following it cannot infer:

const addOrConcat = (a, b) => a + b;

are a and b numbers or strings, both? I've tried this in flow and it compiles, but I'm not sure what the inferred type is, especially for this:

I'd expect it not to compile honestly, at least not without explicit types supplied by the user.. If it's see this in JS (non-typed) code it would seem like an an oversight by the author.

Regarding your second example, an as const on args fixes it:

IIRC the above was a conscious decision to infer [10, 'a'] as (string | number)[] and not [10, 'a'] by default. I think the reason is that usually what you expect is (string | number)[] in this case, since it's not unusual for the following code to appear:

const arr = [10, 'a'];
if (...) {
  arr.push('foo');
}

If you'd infer arr as [number, string], you wouldn't be able to .push() to it later in the code.

I agree with the package.json#types notion. They could've been more nice about it, but I guess what's done is done... I don't think that's a good reason to abandon a whole ecosystem though.

[FezVrasta]

I don't think that's a good reason to abandon a whole ecosystem though.

I don't plan to abandon it, I don't desire to join it at all. I just prefer the soundness of Flow, and the way it handles type inference.

[FezVrasta]

Just yet another example I stumbled upon today:

Object.fromEntries(new URL('http://example.org/?foo=bar').searchParams.entries());

This thing throws 2 errors:

• Property 'fromEntries' does not exist on type 'ObjectConstructor'.ts(2339);
• Property 'entries' does not exist on type 'URLSearchParams'.ts(2339);

How can someone willingly decide to use TypeScript?

[bengry]

This is due to the target + lib property in tsconfig.json - TS tried to warn you about stuff, including calling non-existing methods.
For example:

interface Person {
  getName(): string
}

declare const p: Person;
p.getName(); // compiles
p.getNama(); // errors - `getNama` doesn't exist on type `Person` (notice the typo).

It's the same thing with fromEntries and entries - TS doesn't know these methods exist on the type ObjectConstructor or URLSearchParams, since it takes the definitions for those types based on the target you're on (and the additional lib you have set. If you target ES3, and your lib doesn't include anything newer, than it makes sense that these method won't exist on these objects, if you target to a newer runtime and/or add the proper types to the lib (e.g. ES2019), they should work fine.

I'm not sure how this works on Flow TBH, would be happy to learn though.

[FezVrasta]

I have everything set to ES2019 and it doesn't work, this is the issue. Not even the TypeScript Playground works. This is insane.

{
  "compileOnSave": true,
  "compilerOptions": {
    "strict": true,
    "target": "ES2019",
    "module": "commonjs",
    "outDir": "./build",
    "baseUrl": "./src",
    "sourceMap": true,
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "lib": ["ES2019"],
  },
  "include": [
    "src/**/*"
  ],
  "exclude": [
    "node_modules",
    "**/*.spec.ts"
  ]
}

[bengry]

The playground doesn't allow specifying the lib unfortunately, but with your compilerOptions things work just fine, once I added "DOM", "DOM.Iterable" to the lib: https://codesandbox.io/s/mutable-voice-v38v5?file=/src/index.ts.
The reason "DOM" is needed is because URLSearchParams is not part of ECMAScript, but instead part of the browser API (doing new URLSearchParams() in node wouldn't work), and "DOM.Iterable" because .entries() (and you could even omit it, the URLSearchParams itself is an Iterable) didn't always exist on it, it was added later (relevant MDN).

[FezVrasta]

Wow, thanks, that's crazy, basically every single TS user in the world has a different config then.

This doesn't really explain why Object.fromEntries needs the DOM definitions... It's part of the ECMAScript specification.

Also, it doesn't explain why URLSearchParams is available in ES2019, but its .entries() method needs additional libraries.

Let's be honest, this situation is insane.

[bengry]

Also, it doesn't explain why URLSearchParams is available in ES2019, but its .entries() method needs additional libraries.

This is incorrect, URLSearchParams is unrelated to ES2019, you can have just DOM and get it.

Wow, thanks, that's crazy, basically every single TS user in the world has a different config then.

Not really, it's very configurable, especially since there are many targets that JS can run on, and TS also does transpilation to older targets for some stuff (that's the main difference between target and lib in this case). Not sure that's a good thing, but it's there...

Object.fromEntries doesn't need the DOM definitions, only ES2019.
URLSearchParams needs DOM (since it's part of that API, and not ECMAScript).
URLSearchParams.entries() needs DOM.Iterable since it was added later, as part of the iterable spec (which also makes Maps, Sets, Arrays etc. iterable).

If you want all 3 to work in 1 project - you need all 3 definitions. This is pretty much where this ends for the most part. You can remove any of one them and 1 thing will break, as detailed above.

Let's be honest, this situation is insane.

I'll also mention that The app I'm working on day-to-day has the following in lib: ["dom", "dom.iterable", "es2018", "esnext.array"]. This is pretty much what's in the CodeSandbox I sent above, with the addition of esnext.array to get Array.prototype.flat and Array.prototype.flatMap, since those are not supported everywhere we need just yet (we polyfill them using core-js). We haven't had to change this for over a year or so, and that change was to add esnext.array when we wanted to use flatMap and get type-safety for it. We really don't babysit TS, not at all.

Again, I haven't had much experience with Flow, let alone in a production application, but TS answers what I need from it, is easy to use (for me), and it seems like it has a bigger ecosystem than Flow does, which is what I want - most libraries I use either have built-in .d.ts, or a @types package, unlike Flow. If it were the other way around - I'd probably be using Flow if it would give me enough type-safety like TS does.

[FezVrasta]

This is incorrect, URLSearchParams is unrelated to ES2019, you can have just DOM and get it.

Without the DOM, DOM.Iterable definitions, URLSearchParams is available, but lacks some methods. This is NOT okay.

Object.fromEntries doesn't need the DOM definitions, only ES2019.

Yes it does, with just ES2019 I get the error I reported above.

Again, I haven't had much experience with Flow, let alone in a production application, but TS answers what I need from it, is easy to use (for me), and it seems like it has a bigger ecosystem than Flow does, which is what I want - most libraries I use either have built-in .d.ts, or a @types package, unlike Flow. If it were the other way around - I'd probably be using Flow if it would give me enough type-safety like TS does.

Just because TS is more popular it doesn't mean everybody have to use it, and for sure it doesn't mean I need to migrate a perfectly good Flow-typed project to it.

[bengry]

Without the DOM, DOM.Iterable definitions, URLSearchParams is available, but lacks some methods. This is NOT okay.

Like what? I'll be happy to provide an explanation, if I know it.

Yes it does, with just ES2019 I get the error I reported above.

The error you got was about URLSearchParams maybe? Here's a CodeSandbox with just ES2019 that shows Object.fromEntries works fine.

Just because TS is more popular it doesn't mean everybody have to use it, and for sure it doesn't mean I need to migrate a perfectly good Flow-typed project to it.

Of course not, I just thought that it would be less trouble to maintain, if the majority of users are either using JS (i.e. non-typed) or TS, than you could deprecate the Flow definitions, and not need to maintain any external types (so any code changes in the lib would reflect in the types with no additional effort). This is an assumption of course, but based on the entirety of the ecosystem, I believe it holds true in this case as well. Also, it would allow more people to help maintain this repo, since more people use TypeScript than Flow, that's all.