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
Introduce package for Features #7242
Conversation
bd44f66
to
682ea6f
Compare
packages/technical-features/feature-core/src/registration-of-feature.test.ts
Show resolved
Hide resolved
const registerFeatureRecursed = ( | ||
di: DiContainer, | ||
feature: Feature, | ||
dependedBy?: Feature | ||
) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this is more readable imho + there is no actual reason to use arrow-function, this
context is not even considered).
const registerFeatureRecursed = ( | |
di: DiContainer, | |
feature: Feature, | |
dependedBy?: Feature | |
) => { | |
function registerFeatureRecursed(di: DiContainer, feature: Feature, dependedBy?: Feature) { |
682ea6f
to
275adee
Compare
|
||
#### NPM packages exporting a Feature | ||
- Prefer `peerDependencies` since they are installed from the application and are not allowed to be in the built bundle. | ||
- Prefer exporting `injectionToken` instead of `injectable` for not allowing other features to access technical details like the `injectable` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the example above show this then. Not the best if the best practice is not shown in the only example code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just example which shows how Feature is created. We will have scaffolding for Features which will demonstrate both bullet points. And link for that will be updated here.
Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
275adee
to
5f01374
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This
feature-core
package will allow us to start extending the application without relying current Extension API.Expect future PRs to showcase usage.