-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flow type support #8
Comments
@sebmck Yeah, I'm interested in parsing it (while I'm not sure what are we going to transpile it to). But currently got sick and can't really spend much time on this. If you wish, you can start implementing it in a separate branch so we would merge it when everything's done. |
@RReverser Probably just ignore it completely which is what |
Just came here to ask for this following eslint/eslint#1486 and eslint/eslint#1291 (comment) :) 👍 |
Started this at https://github.com/RReverser/acorn-jsx/tree/flow-types. I'm going to port it over from |
I've committed what I've currently done since I'm not sure if I'll have time to finish it. Currently the tests for interfaces, declare statements and type aliases are commented out. There's some weird behaviour with the |
Looks like most of the @RReverser Should the current ranges be used or do you think that it should be on parity with |
Currently at a crossroad where the only things left are:
@RReverser Any pointers? |
Sorry, got in stuck with other stuff here. Regarding range - I suppose they do it in order to have node to cover both name and type so there's no characters left in between that don't "belong" to any node by location. I believe it's not critical for us and probably even better to be left separately. Ambigous syntax should be definitely fixed. What is the problem with Things like |
Re: ambiguous syntax. Since super classes can be any expression and not just an identifier the super class is parsed as |
Hm... As for now I don't see better ways to handle this than to add |
I've currently got everything implemented, except for the backfix. It's difficult to implement as all the type functions would have to be modified to allow for an optional Sorry about the messy branch history, I'll rebase and submit a PR once I'm done. |
Oh and types within arrow function parameters such as |
If you can't handle some cases, leave them and I'll implement them on my own after PR. |
@RReverser @sebmck What's the status? I see that babel has a flow transformer, so is this solved? If not, how can I help to move this forward? |
@davidpfahler Babel has a Flow plugin but it relies on the Babel Acorn fork. |
@sebmck Btw now that we have both JSX and Flow implemented as plugins - maybe time to add Flow support to this repo? Should be easier to maintain (since I'll be able to update all the methods at once as soon as breaking version of Acorn appears). |
@RReverser It still relies on the lookahead I patched into Acorn. I remain unconvinced that there are performance implications with having lookaheads since Esprima is already much faster and simpler with them. I also still need to update the Babel internal method names to match the new ones that I added to Acorn core. |
@RReverser Sure, I'm actually super keen on getting this into |
@RReverser Would you be interested in supporting flow types? I think it'd be a good feature to add, especially since it can be used in conjunction with JSX. I can give this a go if this is something that you're interested in. Would be aiming to pass the entire esprima-fb type test suite.
The text was updated successfully, but these errors were encountered: