Improve bootstrap logging for help with iframe debugging #4020
Conversation
Nokel81
requested review from
nevalla and
jim-docker
and removed request for
a team
| @@ -43,6 +43,8 @@ export abstract class BaseStore<T> extends Singleton { | |||
| protected storeConfig?: Config<T>; | |||
| protected syncDisposers: Disposer[] = []; | |||
|
|
|||
| readonly displayName: string = this.constructor.name; | |||
There was a problem hiding this comment.
Remember info for class name would be useless/muggled when app is built with webpack's --production=true && optimization.minimized = true
There was a problem hiding this comment.
Yes I know, which is why for all of our built-in T extends BaseStore I override this. I would have wanted to make it abstract but that would have been a breaking change.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Nokel81
force-pushed
the
improve-bootstrap-loggin
branch
from
5a024ca to
d4c2afe
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Nokel81
force-pushed
the
improve-bootstrap-loggin
branch
from
d4c2afe to
82f95f0
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Nokel81
force-pushed
the
improve-bootstrap-loggin
branch
from
82f95f0 to
a0dcf34
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Nokel81
force-pushed
the
improve-bootstrap-loggin
branch
2 times, most recently
from
384c464 to
f1793a2
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Nokel81
force-pushed
the
improve-bootstrap-loggin
branch
from
f1793a2 to
d6fb3fa
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Nokel81
force-pushed
the
improve-bootstrap-loggin
branch
from
d6fb3fa to
3da2dfb
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Nokel81
force-pushed
the
improve-bootstrap-loggin
branch
from
3da2dfb to
24d1295
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
| @@ -66,13 +72,13 @@ export abstract class BaseStore<T> extends Singleton { | |||
| const res: any = this.fromStore(this.storeConfig.store); | |||
|
|
|||
| if (res instanceof Promise || (typeof res === "object" && res && typeof res.then === "function")) { | |||
| console.error(`${this.constructor.name} extends BaseStore<T>'s fromStore method returns a Promise or promise-like object. This is an error and must be fixed.`); | |||
| console.error(`${this.displayName} extends BaseStore<T>'s fromStore method returns a Promise or promise-like object. This is an error and must be fixed.`); | |||
There was a problem hiding this comment.
errors must go through console?
There was a problem hiding this comment.
Do you mean instead of logger? That will be resolved by #4317
| logger.info("[App]: Unload app"); | ||
|
|
||
| window.addEventListener("beforeunload", () => { | ||
| logger.info(`${RootFrame.logPrefix} Unload app`); |
There was a problem hiding this comment.
| logger.info(`${RootFrame.logPrefix} Unload app`); | |
| logger.info(`${RootFrame.logPrefix} Unload root frame`); |
?
There was a problem hiding this comment.
I don't think that is necessary given that logPrefix is already "[ROOT-FRAME]:"
Signed-off-by: Sebastian Malton sebastian@malton.name
This should help with issues like #4018