Skip to content
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

Fix ObjectProperty patterns #5762

Merged
merged 3 commits into from
May 23, 2017
Merged

Fix ObjectProperty patterns #5762

merged 3 commits into from
May 23, 2017

Conversation

haltcase
Copy link
Contributor

Q A
Patch: Bug Fix? yes
Major: Breaking Change?
Minor: New Feature?
Deprecations?
Spec Compliancy? yes
Tests Added/Pass?
Fixed Tickets Fixes #4741
License MIT
Doc PR no
Dependency Changes

Add Patterns to the ObjectProperty validator to fix nested patterns. #5722 has already been merged to fix ArrayPatterns.

Also add RestElement to allow object rest to appear.

Target case:

t.objectPattern([
  t.objectProperty(
    t.identifier('a'),
    t.objectPattern([
      t.objectProperty(
        t.identifier('b'),
        t.stringLiteral('foo')
      )
    ])
  )
])

@mention-bot
Copy link

@citycide, thanks for your PR! By analyzing the history of the files in this pull request, we identified @existentialism, @hzoo and @phantom10111 to be potential reviewers.

@codecov
Copy link

codecov bot commented May 22, 2017

Codecov Report

Merging #5762 into 7.0 will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              7.0    #5762      +/-   ##
==========================================
- Coverage   84.63%   84.62%   -0.02%     
==========================================
  Files         282      282              
  Lines        9854     9854              
  Branches     2766     2766              
==========================================
- Hits         8340     8339       -1     
- Misses       1000     1001       +1     
  Partials      514      514
Impacted Files Coverage Δ
packages/babel-types/src/definitions/core.js 98.21% <ø> (ø) ⬆️
packages/babel-traverse/src/visitors.js 85.71% <0%> (-0.96%) ⬇️

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 853b9f8...e14412e. Read the comment docs.

@hzoo
Copy link
Member

hzoo commented May 22, 2017

Awesome! Can we add a passing test too? https://github.com/babel/babel/blob/master/packages/babel-types/test/validators.js

@existentialism existentialism added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label May 23, 2017
@haltcase
Copy link
Contributor Author

Added a test that includes uses of both the object pattern & the array pattern - let me know if there's any issues with the structure of that test, there just isn't a lot to reference in there yet 😆

@hzoo
Copy link
Member

hzoo commented May 23, 2017

Nope sounds good to me! (just need a test to check that it's not failing)

@hzoo hzoo merged commit 8772e7f into babel:7.0 May 23, 2017
@haltcase haltcase deleted the object-property-patterns branch May 23, 2017 00:31
hzoo pushed a commit that referenced this pull request Jun 8, 2017
* Backport array & object pattern fixes to 6.x

Original PRs merged to 7.0 as #5722 and #5762

* fix lint error
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants