(@fluent/syntax) Migrate to TypeScript

[stasm]

See #376.

[stasm]

@Pike, here's the finished PR migrating @fluent/syntax to TypeScript. Would you have a moment to take a look at it? I expect fluent-syntax/src/ast.ts will be the most interesting, but I'm looking for general feedback too. Thanks!

[Pike]

Generally this looks good, a couple of questions and nits, though.

One thing I only realized in this patch, is the use of .ts files, but imports from .js files. That puzzled me, though VS Code doesn't have a problem resolving those.

Generally, does the introduction of [].includes change the browser compat matrix? Asking for Pontoon.

And there are a few more left-over checks on node.type, I wonder if you want to convert them to instanceof checks, too?

And then maybe this one would be good to go through a feedback loop of a Pontoony, I just noticed that I don't know how they construct Fluent instances in the rich editor.

Also a few nits/questions inline.

[Pike]

What does this do? Once more mostly my curiousity.

[stasm]

It's an index signature. The Visitor needs it because it dynamically accesses methods based on the type of the visited node. I've used unknown so that the subclasses of BaseNode can narrow down the types of their members according to their needs.

[stasm]

Thanks for the review, @Pike!

One thing I only realized in this patch, is the use of .ts files, but imports from .js files. That puzzled me, though VS Code doesn't have a problem resolving those.

TS doesn't touch thte from part of imports. Including .js makes the generated output (in esm/) work in <script type=module> situations. Node as well as Rollup would also accept extensionless paths, which is what most JS/TS projects generally do. FWIW I think this is a bad practice because it precludes browser compatibility from the get go.

Generally, does the introduction of [].includes change the browser compat matrix? Asking for Pontoon.

Good question. Array.prototype.includes was added in Firefox 43 (we support 52+) and Chrome 47 (we support 55+). Old Edge and Safari should also work.

And there are a few more left-over checks on node.type, I wonder if you want to convert them to instanceof checks, too?

Yeah, I think so. instanceof helps TS infer types, which improves the quality of the code and the editing experience.

And then maybe this one would be good to go through a feedback loop of a Pontoony, I just noticed that I don't know how they construct Fluent instances in the rich editor.

Good idea, I'll reach out.

[stasm]

I updated the PR to address your comments. I'd like to look again at the index signatures of Visitor and Transformer. I think the way I declare them right now would make it hard for consumers using TypeScript to keep state on the instances of subclasses. I'll fix this tomorrow.

[stasm]

I relaxed the index signature, documented both the Visitor and the Transformer, and also explicitly listed all possible visit${node} methods to help implementors get their type right.

[Pike]

LGTM. Giving an r+ assuming that the Pontoonies survive ;-)

[stasm]

I've verified in a local instance that @fluent/syntax built from this PR works in Pontoon. I'll merge and release on Monday.