-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: improve ConsolidationEnhancer #7410
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
base: master
Are you sure you want to change the base?
Conversation
f5139bb
to
a3c4345
Compare
BREAKING CHANGE: Certain class members are now static and `disabled` is a potentially-async getter only. **Dynamic Disable** The enhancer can now be dynamically enabled & disabled after being bound. **Static Class Members** Class members that do not require knowledge of the class state are now static. Signed-off-by: Rifa Achrinza <[email protected]>
a3c4345
to
798de8b
Compare
@inject.view(filterByKey(CoreBindings.APPLICATION_CONFIG.key)) | ||
private _config: ContextView<ApplicationConfig>, |
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.
Should move this to property injection.
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.
Are you modifying the _config object afterwards?
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.
ContextView would allow modification of the configuration after the ConsolidationEnhancer is initialised, which would give a better developer experience instead of having to re-bind the enhancer for those who need a more "dynamic" configuration.
@@ -152,22 +177,22 @@ export class ConsolidationEnhancer implements OASEnhancer { | |||
return schema; | |||
} | |||
|
|||
private patchRef( | |||
private static patchRef( |
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.
private static patchRef( | |
private static _patchRef( |
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.
Why changing the name? Do we have a convention to use _
for private declaration names?
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.
Ctrl + Shift + F on vscode shows that this naming convention is being used across several packages:
loopback-next/packages/context/src/binding.ts
Lines 360 to 377 in 1e3f628
private _scope?: BindingScope; | |
/** | |
* Scope of the binding to control how the value is cached/shared | |
*/ | |
public get scope(): BindingScope { | |
// Default to TRANSIENT if not set | |
return this._scope ?? BindingScope.TRANSIENT; | |
} | |
/** | |
* Type of the binding value getter | |
*/ | |
public get type(): BindingType | undefined { | |
return this._source?.type; | |
} | |
private _cache: WeakMap<Context, ValueOrPromise<T>>; | |
private _getValue?: ValueFactory<T>; |
loopback-next/packages/core/src/application.ts
Lines 72 to 81 in 1e3f628
private _isShuttingDown = false; | |
private _shutdownOptions: ShutdownOptions; | |
private _signalListener: (signal: string) => Promise<void>; | |
private _initialized = false; | |
/** | |
* State of the application | |
*/ | |
private _state = 'created'; |
loopback-next/packages/http-server/src/http-server.ts
Lines 89 to 94 in 1e3f628
private _listening = false; | |
private _protocol: HttpProtocol; | |
private _address: string | AddressInfo; | |
private requestListener: RequestListener; | |
readonly server: http.Server | https.Server; | |
private _stoppable: stoppable.StoppableServer; |
This PR should be bundled with other breaking changes.
Dynamic Disable
The enhancer can now be dynamically enabled & disabled after being bound.
This feature is intended to provide flexibility for LoopBack applications to have more "instant" configuration control during runtime without requiring a blocking rebind.
Static Class Members
Class members that do not require knowledge of the class state are now static.
Signed-off-by: Rifa Achrinza [email protected]
Checklist
npm test
passes on your machinepackages/cli
were updatedexamples/*
were updated👉 Check out how to submit a PR 👈