+
Skip to content

Conversation

markwhitfeld
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #276, #213 and #561

Currently the logger writes an action to the console with one group that closes after the action completes. This works great if all actions only do synchronous work but creates a mess in the console as soon as there is asynchronous completion of actions.

What is the new behavior?

If an action will complete asynchronously then the console group for the action is closed after all synchronous work is done and a new group is opened when the asynchronous completion or error happens.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@arturovt arturovt changed the title fix(router-plugin): log async completion in separate group fix(logger-plugin): log async completion in separate group Aug 24, 2019
private getActionLogHeader() {
const actionName = getActionTypeFromInstance(this.action);
const formattedTime = formatTime(this.startedTime);
const message = `action ${actionName} (started @ ${formattedTime})`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is redunant, you can

return `action ${actionName} (started @ ${formattedTime})`;

In addition, I recommend always describing the return type, since in the future you can forget to return an object instead of a returned string.

private getActionLogHeader(): string {
 
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Will do

}

errored(error: any) {
if (this.synchronousWorkEnded) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is synchronousWorkEnded?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActionLogger instance is created when any action is dispatched. As actions can be sync and async this is a flag that basically says "oh I've completed doing my synchronous job".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for some reason this is all very complicated, but oh well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need more comments I guess

await promise;

// Assert
const expectedCallStack = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest also adding a test that will call three asynchronous actions and see how it will look in the logs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea. Especially if they call each other.

@markwhitfeld markwhitfeld changed the title fix(logger-plugin): log async completion in separate group WIP - fix(logger-plugin): log async completion in separate group Sep 16, 2019
@splincode
Copy link
Member

Will we see this in the release?

@markwhitfeld
Copy link
Member Author

No, probably not. This will require quite a bit more code to resolve and I am spending all my available time on reviewing PRs 😄

@markwhitfeld markwhitfeld marked this pull request as draft April 30, 2020 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载