Open macabeus opened 4 years ago
This is a really interesting idea! I think it would improve the developer experience in TypeScript by a lot.
It would be interesting to see a proof-of-concept prototype of this. If you're up for it, I think the best place to start would be @fluent/syntax
's Visitor
class.
@stasm Hey, I just launched the first proof of concept version: https://github.com/macabeus/fluent-typescript-loader 🎉 I would be happy if you could check that =]
At this moment, I'm generating the .d.ts
file for just one .ftl
.
I'll have more work to could generate correctly the types on projects that has more than one .ftl
file, because I'll need to merge that on just one file.
Also we'll need to check if it'll work with fluent-react
.
Anyway, fluent-typescript-loader
already is working on the most simple case.
We would improve types such as { name: string | number }
to be { name: string }
when we add semantic comments
on Fluent.
Very exciting, @macabeus! I can't wait to try this out in a project!
I looked at the implementation, and I'd like to provide some high level feedback:
I would encourage you to not use @fluent/bundle
's AST. It's meant to be internal and is optimized for the memory and performance requirements of the runtime. It might change in the future without notice. See the Internal representation of Pattern
is private point in the @fluent/bundle
0.14 changelog.
Instead, you should be able to use @fluent/syntax
for this, which provides a convenient FluentParser.parse
API, and has a well-defined AST. It's the same package that powers the AST tab in the Fluent Playground.
@fluent/syntax
also provides a Visitor
API which you could use to walk the AST of a message and collect all variable names referenced in it. You'd just need to implement a visitVariableReference
method on a subclass of the visitor. See this test in Gecko for a real-life usage example of this API.
Related to the above: may I suggest to not import directly from the esm
folder?
import { ComplexPattern } from '@fluent/bundle/esm/ast';
As mentioned above, ComplexPattern
isn't meant to be part of the public API. Furthermore, in #480 I'm experimenting with Node's conditional exports. If successful, it will make it impossible to import from specifiers inside the package.
Rather than string | number
, you could declare the arguments as FluentArgument
.
Side note: I might rename FluentArgument
to FluentVariable
in #481. I'll ping you if I do.
Also, +1 to the comment about semantic comments.
Lastly, I really like how you used the id
as a type parameter, which made it possible to type the return value of getMessage
as the generic Message<T>
, which means formatPattern
can know about whihc message the pattern came from. That's neat.
I hope this help. Let me know if you have any questions!
@stasm Hey, I just released a new version: https://github.com/macabeus/fluent-typescript/releases/tag/0.0.3 ! 🎉
Now I think that this tool is so much more stable. Could you take a review on codebase again, please?
Now I want to add support to fluent-react
and react-i18next
.
Nice work, @macabeus! I installed it in a test project, and I was impressed by the results!
It looks like the project is maturing nicely. I have two pieces of feedback, but overall it looks great. I hope more people will start using it!
I didn't understand at first that fluent-typescript
starts a watcher. I ran it, it said Ready
, and... nothing happened :) I had to edit one of the Fluent files in my project for it to do its things. Perhaps consider adding a non-watcher mode to the CLI command?
Good work implementing a Visitor
. As a slight improvment, rather than having two visitor classes you could try having a single class with both visitMessage
and visitVariableReference
methods. In visitMessage
you could set this.currentMessageId
and this.currentVariableReferences = new Set();
, and then call this.genericVisit()
to descend into the message, including any references in its patterns. But like I said, this would be just a small improvement; no need to do it if you prefer the current approach.
Nice work!
@stasm I was very busy on the last couple of weeks, but finally I added support to @fluent/react
as well as react-i18next
, and I added auto-emit when start to watch.
Now it's a little more useful CLI tool to use with Fluent =]
https://github.com/macabeus/fluent-typescript/releases/tag/0.0.4
Currently we could define messages with variables:
and generating a bundle, we could use that on our code:
But since there are lacking type informations, TS can't check if I'm writing all variables that message requires, or if I typed something wrong, as well as it's missing autocomplete.
We could improve even more if we could read the comments
On this small example, a valid type declaration would be: