Skip to content
This repository has been archived by the owner on May 19, 2018. It is now read-only.

Add support for Flow declare export (default) #441

Closed
wants to merge 3 commits into from

Conversation

jamiebuilds
Copy link
Contributor

Q A
Bug fix? no
Breaking change? no
New feature? yes
Deprecations? no
Spec compliancy? sorta?
Tests added/pass? yes
Fixed tickets N/A
License MIT

Adds support for the following Flow syntaxes:

declare module A { declare export default {}; }
declare module A { declare export default number; }
declare module A { declare export default var val: number; }
declare module A { declare export default function method(): number; }

declare module A { declare export var val: number; }
declare module A { declare export function method(): number; }

cc @samwgoldman

@jamiebuilds jamiebuilds requested review from danez and jeffmo March 30, 2017 00:44
@codecov
Copy link

codecov bot commented Mar 30, 2017

Codecov Report

Merging #441 into master will decrease coverage by 1.23%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
- Coverage   98.25%   97.02%   -1.24%     
==========================================
  Files          20       20              
  Lines        3504     3529      +25     
  Branches      927      932       +5     
==========================================
- Hits         3443     3424      -19     
- Misses         22       54      +32     
- Partials       39       51      +12
Flag Coverage Δ
#babel ?
#babylon 97.02% <100%> (+0.02%) ⬆️
Impacted Files Coverage Δ
src/plugins/flow.js 97.3% <100%> (-0.89%) ⬇️
src/parser/expression.js 93.84% <0%> (-3.54%) ⬇️
src/tokenizer/state.js 97.22% <0%> (-2.78%) ⬇️
src/parser/index.js 97.29% <0%> (-2.71%) ⬇️
src/tokenizer/index.js 97.31% <0%> (-1.04%) ⬇️
src/parser/statement.js 98.15% <0%> (-0.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f3123c...bd4afe2. Read the comment docs.

@@ -105,28 +105,44 @@ pp.flowParseDeclareFunction = function (node) {
return this.finishNode(node, "DeclareFunction");
};

pp.flowParseDeclare = function (node) {
if (this.match(tt._class)) {
let flowParseDeclareOrType = function(node, allowModule, allowType) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a huge fan of how I did this (especially since node is potentially unused), but otherwise I'd be repeating a bunch of stuff.

@jamiebuilds
Copy link
Contributor Author

Test failures don't seem to be a result of this PR

@jamiebuilds
Copy link
Contributor Author

Note: The generated AST just copies what Flow has.

@hzoo
Copy link
Member

hzoo commented Mar 30, 2017

Hey @thejameskyle! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

@danez
Copy link
Member

danez commented Mar 30, 2017

Duplicate of #224

@danez danez closed this Apr 21, 2017
@danez danez deleted the flow-declare-exports branch April 21, 2017 12:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants