Make generate-js.js ts clean #430
Conversation
|
I think this is the next one I want to land. I'll look at it later today, but could you do a quick rebase please? |
|
Rebased |
|
JFYI there's a full rewrite in TS here Feel free to steal anything from there, as I don't have time to update it to most recent version |
|
@reverofevil Nice. From my point of view a full typescript rewrite would be much better. But I'd assumed that sticking with code that "just works" without a build step was probably a project requirement. If thats not the case, then sure... |
|
@markw65 Whether it's a full rewrite or not, we still have to figure out how to make types work, and it's the same in either case. As far as I understand, you plan to continue cleaning up peggy's codebase. In that case feel free to take anything from a (differently) cleaned up version of peggy. |
|
@hildjj Is there anything else you want here? I have the changes to stack.js if you'd like me to roll those into this pull request; or I can do that separately. |
|
@hildjj ping |
|
Sorry, have been traveling. Will get to this when I can, it's still first on the list. |
|
And of course I didn't get to this in a timely fashion. Apologies. Looking at it now. |
|
All this needs is a CHANGELOG.md entry, and it's ready to go. |
While working on #429 I found that having @ts-check enabled on the new files was very helpful, so I decided to see how hard it would be to turn on for generate-js.js. This is the result.
I found several typing errors (either errors in jsdoc comments, or in peg.d.ts) - but these were just incorrect declarations - the code itself was fine (also, most of them were introduced by me, when I added source map support to generate-js.js).
I also found one possible bug (again, it was me that did it): A SourceNode was constructed with a child array consisting of strings and an array of SourceNodes (ie
Array<string|Array<SourceNode>>). The type info says it should be aArray<string|SourceNode>. Apparently the code did work though, so who knows.In any case, if this seems worthwhile, I'll be happy to put up similar pull requests for other files as I get time.