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

Add value method to ICapturedArguments. #832

Merged
merged 1 commit into from
Jan 18, 2019

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Jun 19, 2018

This seems to have been an oversight (likely hasn't mattered to date?), both ICapturedNamedArguments and ICapturedPositionalArguments already support .value(). This brings ICapturedArguments in line.

Supports changes in emberjs/ember.js#16747.

@krisselden
Copy link
Contributor

@rwjblue you need to update the Checker<> type with this change.

This seems to have been an oversight (likely hasn't mattered to date?),
both `ICapturedNamedArguments` and `ICapturedPositionalArguments`
already support `.value()`. This brings `ICapturedArguments` in line.
@rwjblue
Copy link
Member Author

rwjblue commented Jan 16, 2019

Updated to make the checker pass, and pinged @chancancode and @krisselden for review. If there is a better way, I'm happy to iterate...

@chancancode
Copy link
Contributor

seems fine to me. not directly related, but we used to do more hand-crafted tuning for the empty cases because we believed them to be common (e.g. value could return the same const empty object every time), did we decide it's not worth it?

@rwjblue rwjblue merged commit 543e767 into glimmerjs:master Jan 18, 2019
@rwjblue rwjblue deleted the add-arguments-value branch January 18, 2019 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants