Skip to content

Commit 9cf5b38

Browse files
committed
feat(autofix-result): refactor logstream handling with modifier
Replace manual logstream management in AutofixResult component with a new `logstream-did-update` modifier that handles subscription lifecycle automatically. This simplifies the component by removing explicit setup, teardown, and state syncing of Logstream instances. Update the template to use the modifier for logstream updates and track logstream content reactively. This ensures better separation of concerns and reduces boilerplate. Add `logstream-did-update` modifier that encapsulates subscribing and unsubscribing to logstreams and invokes a callback on updates. Overall, these changes improve code clarity, correctness, and maintainability around logstream update handling.
1 parent 63bce65 commit 9cf5b38

File tree

3 files changed

+56
-35
lines changed

3 files changed

+56
-35
lines changed

app/components/course-page/test-results-bar/autofix-section/autofix-result.hbs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,9 @@
3535

3636
{{#if (eq @autofixRequest.status "in_progress")}}
3737
<div class="bg-gray-800 border border-gray-700 rounded-sm overflow-y-auto relative grow">
38-
<div
39-
class="p-3"
40-
{{did-insert this.handleMarkdownStreamElementInserted}}
41-
{{did-update this.handleDidUpdateAutofixRequestLogstreamId @autofixRequest.logstreamId}}
42-
{{will-destroy this.handleWillDestroyMarkdownStreamElement}}
43-
>
44-
{{#if this.logstream}}
45-
<AnsiStream @content={{this.logstream.content}} class="text-xs whitespace-pre-wrap" />
38+
<div class="p-3" {{logstream-did-update this.handleLogstreamDidUpdate @autofixRequest.logstreamId}}>
39+
{{#if this.logstreamContent}}
40+
<AnsiStream @content={{this.logstreamContent}} class="text-xs whitespace-pre-wrap" />
4641
{{/if}}
4742
</div>
4843
</div>

app/components/course-page/test-results-bar/autofix-section/autofix-result.ts

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import AnalyticsEventTrackerService from 'codecrafters-frontend/services/analytics-event-tracker';
2-
import type Store from '@ember-data/store';
32
import { action } from '@ember/object';
43
import { service } from '@ember/service';
54
import Component from '@glimmer/component';
65
import { tracked } from '@glimmer/tracking';
7-
import Logstream from 'codecrafters-frontend/utils/logstream';
86
import type AutofixRequestModel from 'codecrafters-frontend/models/autofix-request';
9-
import type ActionCableConsumerService from 'codecrafters-frontend/services/action-cable-consumer';
7+
import type Logstream from 'codecrafters-frontend/utils/logstream';
108

119
interface Signature {
1210
Element: HTMLDivElement;
@@ -17,40 +15,21 @@ interface Signature {
1715
}
1816

1917
export default class AutofixResult extends Component<Signature> {
20-
@service declare store: Store;
21-
@service declare actionCableConsumer: ActionCableConsumerService;
2218
@service declare analyticsEventTracker: AnalyticsEventTrackerService;
2319

24-
@tracked logstream: Logstream | null = null;
2520
@tracked shouldShowFullLog = false;
21+
@tracked logstreamContent: string | null = null;
2622
@tracked diffIsBlurred = true;
2723

2824
@action
29-
handleDidUpdateAutofixRequestLogstreamId() {
30-
if (this.logstream && this.args.autofixRequest.logstreamId !== this.logstream.logstreamId) {
31-
this.logstream.unsubscribe();
32-
this.handleMarkdownStreamElementInserted(); // create a new logstream
33-
}
34-
}
25+
handleLogstreamDidUpdate(logstream: Logstream): void {
26+
this.logstreamContent = logstream.content;
3527

36-
@action
37-
handleLogstreamDidPoll(): void {
28+
// TODO: See if we're doing this too often?
29+
// Ensure we also reload the autofix request to see whether the status has changed
3830
this.args.autofixRequest.reload();
3931
}
4032

41-
@action
42-
handleMarkdownStreamElementInserted() {
43-
this.logstream = new Logstream(this.args.autofixRequest.logstreamId, this.actionCableConsumer, this.store, this.handleLogstreamDidPoll);
44-
this.logstream.subscribe();
45-
}
46-
47-
@action
48-
handleWillDestroyMarkdownStreamElement() {
49-
if (this.logstream) {
50-
this.logstream.unsubscribe();
51-
}
52-
}
53-
5433
@action
5534
toggleBlur() {
5635
if (this.diffIsBlurred) {
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Invokes a callback when a logstream is updated.
2+
// Usage: <div {{logstream-did-update this.handleLogstreamDidUpdate logstreamId}}></div>
3+
import type ActionCableConsumerService from 'codecrafters-frontend/services/action-cable-consumer';
4+
import type Store from '@ember-data/store';
5+
import Logstream from 'codecrafters-frontend/utils/logstream';
6+
import Modifier from 'ember-modifier';
7+
import { inject as service } from '@ember/service';
8+
import { registerDestructor } from '@ember/destroyable';
9+
import { action } from '@ember/object';
10+
11+
interface Signature {
12+
Args: {
13+
Positional: [(logstream: Logstream) => void, string];
14+
};
15+
}
16+
17+
export default class LogstreamDidUpdateModifier extends Modifier<Signature> {
18+
@service declare actionCableConsumer: ActionCableConsumerService;
19+
@service declare store: Store;
20+
21+
callback?: (logstream: Logstream) => void;
22+
logstream?: Logstream;
23+
24+
@action
25+
handleLogstreamDidPoll(): void {
26+
this.callback!(this.logstream!);
27+
}
28+
29+
modify(_element: HTMLElement, [callback, logstreamId]: Signature['Args']['Positional']) {
30+
this.logstream = new Logstream(logstreamId, this.actionCableConsumer, this.store, this.handleLogstreamDidPoll);
31+
this.callback = callback;
32+
33+
console.log(`subscribing to logstream#${logstreamId}`);
34+
this.logstream.subscribe();
35+
36+
registerDestructor(this, () => {
37+
console.log(`unsubscribing from logstream#${logstreamId}`);
38+
this.logstream?.unsubscribe();
39+
});
40+
}
41+
}
42+
43+
declare module '@glint/environment-ember-loose/registry' {
44+
export default interface Registry {
45+
'logstream-did-update': typeof LogstreamDidUpdateModifier;
46+
}
47+
}

0 commit comments

Comments
 (0)