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

[schema] Allow RefResources to have commas in the labels #48

Merged
merged 2 commits into from
Aug 14, 2019

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Aug 9, 2019

This brings the contact.id.firstName,lastName dynamic dropdown pattern to CLI! I've confirmed it works:

contact.id.lastName,firstName,lastName:

This involves couple of changes:

  1. change resource property to KeySchema since dynamic: 'contact' is an error in the editor
  2. That allows us to make more assertions about the shape of actual references, which needs the shape x.y(.z)?. We also now include an optional comma clause (but only 1, I got nervous the regex would get unwieldy. We could also simplify it and just allow commas anywhere, but I worry someone will mess it up and it'll be very unclear why their dynamic dropdown isn't working)

Very strictly speaking, this is a semver.major since before it was valid to have dynamic: 'x' and on this version, it won't be. That said, that wouldn't have produced a working app anyway, so i don't feel too bad about including this in a lesser release if it comes to that.


Note that I'm OOO next week, so if this is ✅, I'd appreciate if you merged it for me. Thanks!

@@ -34,8 +34,11 @@
}
},
"lint-staged": {
"*.{js,json}": [
"*.{js}": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unrelated change, but there are apparently errors if you try to run eslint on json ¯\_(ツ)_/¯

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Tested locally, it works like a charm! I agree this can be a bug fix instead of a breaking change.

@eliangcs eliangcs merged commit 68995bc into master Aug 14, 2019
@eliangcs eliangcs deleted the ref-label-commas branch August 14, 2019 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants