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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(context): allow @config.* to specify the target binding key #3329

Merged
merged 1 commit into from Jul 25, 2019

Conversation

@raymondfeng
Copy link
Member

commented Jul 9, 2019

Sometimes we want to inject configuration from another binding instead of
the current one.

This PR allows the following config injections:

@config({targetBindingKey: 'parent', propertyPath: 'child1'})
@config.getter({targetBindingKey: 'parent', propertyPath: 'child2'})
@config({targetBindingKey: 'anotherBinding'})

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

@raymondfeng raymondfeng requested a review from bajtos as a code owner Jul 9, 2019

@raymondfeng raymondfeng referenced this pull request Jul 9, 2019
3 of 7 tasks complete

@raymondfeng raymondfeng force-pushed the improve-config-injection branch from f2af6aa to 01ef75b Jul 9, 2019

@raymondfeng

This comment has been minimized.

Copy link
Member Author

commented Jul 11, 2019

@bajtos PTAL.

@bajtos
Copy link
Member

left a comment

Nice feature!

Let's improve the details & implementation a bit.

packages/context/src/inject-config.ts Outdated Show resolved Hide resolved
packages/context/src/inject-config.ts Outdated Show resolved Hide resolved
packages/context/src/inject-config.ts Outdated Show resolved Hide resolved

@raymondfeng raymondfeng force-pushed the improve-config-injection branch from 01ef75b to 5cba367 Jul 18, 2019

@raymondfeng

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2019

@bajtos PTAL.

@raymondfeng raymondfeng force-pushed the improve-config-injection branch from 5cba367 to 79ff9fe Jul 23, 2019

@raymondfeng

This comment has been minimized.

Copy link
Member Author

commented Jul 23, 2019

@bajtos LGTY now?

@bajtos

bajtos approved these changes Jul 25, 2019

Copy link
Member

left a comment

The new version looks much better now, I have few more comments to consider.

@raymondfeng raymondfeng force-pushed the improve-config-injection branch 3 times, most recently from 78b38c9 to 08ac4ea Jul 25, 2019

feat(context): allow @config.* to specify the target binding key
Sometimes we want to inject configuration from another binding instead of
the current one.

@raymondfeng raymondfeng force-pushed the improve-config-injection branch from 08ac4ea to eddad57 Jul 25, 2019

@raymondfeng raymondfeng merged commit 42b7b98 into master Jul 25, 2019

4 checks passed

clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 91.545%
Details

@delete-merged-branch delete-merged-branch bot deleted the improve-config-injection branch Jul 25, 2019

@bajtos bajtos referenced this pull request Jul 29, 2019
3 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.