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

Proposal: Breaking change of beforeCreate #490

Closed
clebert opened this issue May 10, 2019 · 3 comments · Fixed by #506
Closed

Proposal: Breaking change of beforeCreate #490

clebert opened this issue May 10, 2019 · 3 comments · Fixed by #506
Assignees
Milestone

Comments

@clebert
Copy link
Member

clebert commented May 10, 2019

Maybe it would be better to pass the whole environment to beforeCreate instead of just the bound feature services. The environment is also the argument for the create method and contains potentially useful information.

New:

readonly beforeCreate?: (
  consumerUid: string,
  env: FeatureAppEnvironment<TConfig, TInstanceConfig, TFeatureServices>
) => void;

Current:

readonly beforeCreate?: (
  consumerUid: string,
  featureServices: FeatureServices
) => void;
@clebert clebert added this to the 2.0 milestone May 10, 2019
@unstubbable
Copy link
Member

🤔 I kind of like the symmetry with the create method, but practically this has no advantage for the integrator since he provides all of the other four props (config, instanceConfig, baseUrl, and idSpecifier) directly.

@clebert
Copy link
Member Author

clebert commented May 12, 2019

But we only give the integrator the consumerUid. Which is a combination of the Feature App ID and the ID specifier. The method of combination is more or less an implementation detail. So there is no intended way for the integrator to get to the configs, etc. Because he doesn't know the Feature App ID yet.

@clebert
Copy link
Member Author

clebert commented May 12, 2019

Perhaps we should offer helper methods to combine or separate IDs?

unstubbable added a commit that referenced this issue Jun 10, 2019
fixes #490

BREAKING CHANGE: Instead of passing the `featureAppId` and the
`featureServices` as separate arguments to the `beforeCreate` callback,
the full Feature App environment, that contains the `featureAppId` and
`featureServices`, is now passed as a single argument (`env`). This is
the same argument that is passed to the Feature App's `create` method.
unstubbable added a commit that referenced this issue Jun 11, 2019
fixes #490

BREAKING CHANGE: Instead of passing the `featureAppId` and the
`featureServices` as separate arguments to the `beforeCreate` callback,
the full Feature App environment, that contains the `featureAppId` and
`featureServices`, is now passed as a single argument (`env`). This is
the same argument that is passed to the Feature App's `create` method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants