feat(core): integrated bootstrap #486
Conversation
| */ | ||
| public abstract main: () => Promise<any>; | ||
|
|
||
| public mainWrapper = (): void => { |
There was a problem hiding this comment.
You use different ways to define methods in this file, I think we should keep only one.
Could we have
public mainWrapper(): void { ... }
instead of
public mainWrapper = (): void => { ... }
There was a problem hiding this comment.
I changed to a lambda to capture "this", otherwise all calls to this would fail because they're made through a callback
There was a problem hiding this comment.
But I have in mind that there was another way to do it, just can't remember it though :p
| /** | ||
| * Initialize by configuring HMR if it is enabled or normally instead. | ||
| */ | ||
| protected initialize = (): void => { |
There was a problem hiding this comment.
same here about the definition of the method
| * Reference: https://github.com/gdi2290/angular-hmr | ||
| * @ignore | ||
| */ | ||
| protected bootstrapHmr: Function = (module: any, bootstrap: () => Promise<NgModuleRef<any>>) => { |
There was a problem hiding this comment.
same here about the definition of the method
| * @ignore | ||
| */ | ||
| protected bootstrapHmr: Function = (module: any, bootstrap: () => Promise<NgModuleRef<any>>) => { | ||
| if(ENV === "development") { |
There was a problem hiding this comment.
This is already checked at line 51. Maybe could we remove this check ? 😊
There was a problem hiding this comment.
Depends on how intelligent Webpack is for dead code removal. I didn't check the generated code though, I've added the check in the function just be be sure that the code would be dropped in production builds
There was a problem hiding this comment.
Indeed, as @dsebastien said, this is needed so that webpack can remove the code inside this function.
The other check (in line 51) makes sure that this function will not be called, but yet the function will still be there and so code that calls the angularclass/hmr module. Therefore the check here is important so webpack can remove the call to angularclass/hmr and also the import ;)
| * Adapt the configuration based on the current environment | ||
| * @param moduleRef - NgModule instance created by Angular for a given platform. | ||
| */ | ||
| public decorateModule = (moduleRef: NgModuleRef<any>): NgModuleRef<any> => { |
There was a problem hiding this comment.
same here about the definition of the method
b2ef7b2 to
97ca29e
Compare
Closes #112 #412
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #112 and #412
What is the new behavior?
Bootstrap supported and HMR integrated in stark-core
Does this PR introduce a breaking change?
Other information