Typecheck #55
Typecheck #55
Conversation
Commit b8df491 added "support for storing white space and comments in the AST"; however, this feature wasn't really documented, tested, and--based on some early experiments typechecking the source--may be ill-typed and/or contain bugs. Since the feature does not seem complete, this commit reverts the change so that some other refactorings and cleanups can take place first.
A few while-loops were actually for-loops in disguise, and some were actually do-loops in disguise. These are a bit easier to reason about than a while loop with breaks and returns placed throughout.
We can name functions directly rather than by binding them to variable names.
This commit adds Flow typechecking. A number of comments have been added to the
source code to aid Flow's typechecking along with notes describing conditions
Flow cannot check. Two fairly mechanical transformations have also been applied:
1. Flow doesn't handle adding properties to object literals that were
previously constructed with properties, which is something we do in the
parse code today. We do this not only with properties that can be absent
(such as default values), but even when building up required properties.
We can work around this issue by always constructing an empty object
literal and adding properties to it. See here for more:
facebook/flow#1606
2. A related issue: Flow doesn't handle adding properties to objects returned
from functions very well, either; which is something the `definition`
function does. It parses any extended attributes, then dispatches to the
appropriate parse code, then attaches those extended attributes to the
result. Now, we thread the extended attributes through to the parse code
and let those functions assign the property.
Rather than mutate its argument, identifiers now returns an array. This is easier to typecheck and reason about.
This commit adds Flow type declarations and annotations for data types produced and consumed by WebIDL2.js. All of the AST data types are prefixed with "IDL". In order to aid the typechecker, a few simple refactorings were performed; however, there were some functions that could not be typechecked completely without a significant refactoring. There is also still plenty of opportunity to refine the AST type declarations.
This commit adds JSDoc documentation. It doesn't really invest too much in descriptions, since that would duplicate work with the README.md. Currently, we also duplicate work with the Flow type declarations. Perhaps we could write, e.g., WebIDL once and generate both the Flow type declarations and JSDoc documentation at the same time ;-)
The same caveat applies to the TypeScript type declarations as they do to the JSDoc declarations: it feels like we are duplicating effort when perhaps we could write once and generate each.
These will be used to auto-generate Flow, JSDoc, and TypeScript declarations.
A new command, `npm run generate-typescript`, can be used to generate TypeScript type declarations from the WebIDL description of webidl2.js.
A new command, `npm run generate-flow`, can be used to generate Flow type declarations from the WebIDL description of webidl2.js.
A new command, `npm run generate-flow`, can be used to generate JSDoc type declarations from the WebIDL description of webidl2.js.
This commit adds two new commands, `npm run generate` and `npm run build`. The former runs the commands for auto-generating Flow, JSDoc, and TypeScript type declarations. The latter runs the auto-generation commands, typechecks the source according to the auto-generated type declarations, and finally generates JSDoc documentation.
|
I have pushed a few more changes in order to cut down on the duplication between the Flow, JSDoc, and TypeScript type declarations. I'm using (abusing?) WebIDL to declare webidl2.js's types, and then I use webidl2.js itself to parse the WebIDL and print out Flow, JSDoc, and TypeScript declarations. WebIDL is not expressive enough (AFAICT) to represent types like |
|
Hi @markandrus, sorry for the delay. I was waiting to get admin on this repo before responding (just got it!). I think this is a pretty cool idea... give me a few more days to look at it. Also, would you be willing to help me with maintenance? The code for this project was written pre-ES6, so it could use a lot of love and modernization. Having said that, it does a really good job at parsing WebIDL. Anyway, if you are willing to commit to helping maintain this - and are willing to support any questions I might have wrt Flow (I've never used Flow - and am always a little scared of adding deps that could go away when the next hotness comes along) - then we can look at merging this. |
|
Hi @marcoscaceres, thanks for taking a look!
Assuming the maintenance is low-traffic, then yes I think I could commit, e.g., to reviewing PRs every now and again. I hesitate to commit to much more than that since my time is pretty stretched these days.
Yes, I was wondering, for example, could we break some of the functionality out into modules (would probably imply introducing a build step like Browserify for browsers), improving the tests, etc. Are there other tasks you've been thinking of?
FWIW, I'm not 100% sure Flow is the best choice going forward. I'm still relatively new to both TypeScript and Flow. I opted for Flow when introducing types here, since I knew they could be included inline in comments (or that entire sections could be skipped with, e.g., Best, |
We generally only update things once of twice a year. WebIDL is pretty stable and rarely gains new features.
Exactly those:) And the coding style is very "early NodeJS" and super hard to decipher, lint, etc. This could do with some more declarative functional love.
So, I'm a bit worried about using things like TypeScript because generally those kinds of projects die after a while (been burned way too many times over last 20 years). I'd like to keep it vanilla - specially because this project only gets a few updates every few years and WebIDL is fairly stable. I'm hopeful that eventually TC-39 will come to its senses and add standardized types into JS... hopefully based on TypeScript, or Flow, the like, given that they've proven tremendously useful. Having said that, I really appreciate that you took the time to experiment adding types. |
Hi,
I find this library very useful, and I hope it will continue to be improved! The
README.mdwas invaluable when consuming the AST, but I found myself wishing for type declarations, for example, to ensure I was handling every possible case. Also, as I began looking intowebidl2.js's source, I began to wonder if the code was indeed producing what theREADME.mdclaimed (without any comments, I found the code pretty difficult to read and I even spotted a few bugs).So I made a few small cleanups, completed a first round of typechecking with Flow, then added Flow type declarations for the AST, then JSDoc, and finally TypeScript. (Please see the commit messages for information on these.) I hope these changes will ease consuming
webidl2.jsas well as development ofwebidl2.js. Here's an example of using the TypeScript declarations with YouCompleteMe:Would you consider such changes and additions?
Mark