This repository has been archived by the owner. It is now read-only.

Check for duplicate named exports in exported destructuring assignments #144

Merged
merged 3 commits into from Oct 4, 2016

Conversation

Projects
None yet
5 participants
@kaicataldo
Copy link
Member

kaicataldo commented Sep 24, 2016

This is a follow-up to #139. I did a little refactoring - passing isDefault all the way through seems unnecessary, in retrospect. Thoughts on this approach? Any cases I missed?

@kaicataldo kaicataldo force-pushed the kaicataldo:check-destructured-exports branch 2 times, most recently from da09c40 to 6800abc Sep 24, 2016

@@ -1,2 +1,11 @@
export const { rhythm } = typography;
export const { TypographyStyle } = typography;
export const { foo } = bar;

This comment has been minimized.

@kaicataldo

kaicataldo Sep 24, 2016

Author Member

These are all unique identifiers and should parse without error

This comment has been minimized.

@hzoo

hzoo Sep 26, 2016

Member

Cool was wondering about a passing test 👍

@kaicataldo kaicataldo force-pushed the kaicataldo:check-destructured-exports branch from 93c3d0d to 5152905 Sep 26, 2016

@@ -909,23 +909,21 @@ pp.checkExport = function (node, checkNames, isDefault) {
// Check for duplicate exports
if (isDefault) {
// Default exports
this.checkDuplicateExports(node, "default", isDefault);
this.checkDuplicateExports(node, "default");
} else if (node.specifiers && node.specifiers.length) {
// Named exports
for (let specifier of node.specifiers) {
const name = specifier.exported.name;
if (name === "default") isDefault = true;

This comment has been minimized.

@loganfsmyth

loganfsmyth Sep 26, 2016

Member

Looks like this isn't needed.

@danez danez added the Tag: Bug Fix label Sep 26, 2016

@danez
Copy link
Member

danez left a comment

Looks good to me, except one minor nitpick.

Could you also please rebase your changes onto master, because currently travis is not working in this branch, but i fixed it in master.

this.checkDeclaration(elem);
}
} else if (node.type === "ObjectProperty") {
this.checkDeclaration(node.value);

This comment has been minimized.

@danez

danez Sep 26, 2016

Member

to much indentation

@kaicataldo kaicataldo force-pushed the kaicataldo:check-destructured-exports branch from 235398a to 909b19a Sep 26, 2016

@kaicataldo

This comment has been minimized.

Copy link
Member Author

kaicataldo commented Sep 26, 2016

Updated - thanks!

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Sep 26, 2016

Current coverage is 94.44% (diff: 94.11%)

Merging #144 into master will decrease coverage by 0.02%

@@             master       #144   diff @@
==========================================
  Files            19         19          
  Lines          3111       3117     +6   
  Methods         327        328     +1   
  Messages          0          0          
  Branches        818        820     +2   
==========================================
+ Hits           2939       2944     +5   
  Misses           94         94          
- Partials         78         79     +1   

Powered by Codecov. Last update 4115bcb...909b19a

@danez

danez approved these changes Sep 26, 2016

@kaicataldo

This comment has been minimized.

Copy link
Member Author

kaicataldo commented Sep 27, 2016

@danez I know we spoke in Slack, but wasn't sure if there is anything I need to do for the failing codecov checks?

@danez

danez approved these changes Oct 4, 2016

@danez danez merged commit 76e6927 into babel:master Oct 4, 2016

1 of 3 checks passed

codecov/patch 94.11% of diff hit (target 94.47%)
Details
codecov/project 94.44% (-0.03%) compared to 4115bcb
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kaicataldo kaicataldo deleted the kaicataldo:check-destructured-exports branch Oct 4, 2016

kristofdegrave pushed a commit to kristofdegrave/babylon that referenced this pull request Oct 27, 2016

Check for duplicate named exports in exported destructuring assignmen…
…ts (babel#144)

* Check for duplicate named exports in exported destructuring assignments

* Refactor duplicate error reporting

* Remove unnecessary check
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.