Skip to content

Commit 34fe433

Browse files
authored
fix: reduce RxJS imported stuff (#1982)
1 parent af21eb7 commit 34fe433

File tree

5 files changed

+70
-78
lines changed

5 files changed

+70
-78
lines changed

projects/ngx-quill/src/lib/quill-editor.component.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ describe('Basic QuillEditorComponent', () => {
211211
await fixture.whenStable()
212212
const spy = spyOn(fixture.componentInstance.quillEditor, 'off').and.callThrough()
213213

214-
fixture.componentInstance.ngOnDestroy()
214+
fixture.destroy()
215215

216216
expect(spy).toHaveBeenCalledTimes(3)
217217
const quillEditor: any = fixture.componentInstance.quillEditor
@@ -1007,7 +1007,7 @@ describe('Advanced QuillEditorComponent', () => {
10071007
fixture.destroy()
10081008

10091009
expect(quillOffSpy).toHaveBeenCalledTimes(3)
1010-
expect(editorFixture.componentInstance.subscription).toEqual(null)
1010+
expect(editorFixture.componentInstance.eventsSubscription).toEqual(null)
10111011
expect(quillOffSpy).toHaveBeenCalledWith('text-change', jasmine.any(Function))
10121012
expect(quillOffSpy).toHaveBeenCalledWith('editor-change', jasmine.any(Function))
10131013
expect(quillOffSpy).toHaveBeenCalledWith('selection-change', jasmine.any(Function))

projects/ngx-quill/src/lib/quill-editor.component.ts

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { DOCUMENT, isPlatformServer } from '@angular/common'
1+
import { isPlatformServer } from '@angular/common'
22
import { DomSanitizer } from '@angular/platform-browser'
33

44
import type QuillType from 'quill'
@@ -18,7 +18,6 @@ import {
1818
input,
1919
NgZone,
2020
OnChanges,
21-
OnDestroy,
2221
OnInit,
2322
Output,
2423
PLATFORM_ID,
@@ -77,7 +76,7 @@ export type EditorChangeContent = ContentChange & { event: 'text-change' }
7776
export type EditorChangeSelection = SelectionChange & { event: 'selection-change' }
7877

7978
@Directive()
80-
export abstract class QuillEditorBase implements AfterViewInit, ControlValueAccessor, OnChanges, OnInit, OnDestroy, Validator {
79+
export abstract class QuillEditorBase implements AfterViewInit, ControlValueAccessor, OnChanges, OnInit, Validator {
8180
readonly format = input<'object' | 'html' | 'text' | 'json' | undefined>(
8281
undefined
8382
)
@@ -142,11 +141,10 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce
142141
onModelTouched: () => void
143142
onValidatorChanged: () => void
144143

145-
private subscription: Subscription | null = null
144+
private eventsSubscription: Subscription | null = null
146145
private quillSubscription: Subscription | null = null
147146

148147
private elementRef = inject(ElementRef)
149-
private document = inject(DOCUMENT)
150148

151149
private cd = inject(ChangeDetectorRef)
152150
private domSanitizer = inject(DomSanitizer)
@@ -156,6 +154,15 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce
156154
private service = inject(QuillService)
157155
private destroyRef = inject(DestroyRef)
158156

157+
constructor() {
158+
this.destroyRef.onDestroy(() => {
159+
this.dispose()
160+
161+
this.quillSubscription?.unsubscribe()
162+
this.quillSubscription = null
163+
})
164+
}
165+
159166
static normalizeClassNames(classes: string): string[] {
160167
const classList = classes.trim().split(' ')
161168
return classList.reduce((prev: string[], cur: string) => {
@@ -264,7 +271,8 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce
264271

265272
let bounds = this.bounds() && this.bounds() === 'self' ? this.editorElem : this.bounds()
266273
if (!bounds) {
267-
bounds = this.service.config.bounds ? this.service.config.bounds : this.document.body
274+
// Can use global `document` because we execute this only in the browser.
275+
bounds = this.service.config.bounds ? this.service.config.bounds : document.body
268276
}
269277

270278
let debug = this.debug()
@@ -490,13 +498,6 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce
490498
}
491499
}
492500

493-
ngOnDestroy() {
494-
this.dispose()
495-
496-
this.quillSubscription?.unsubscribe()
497-
this.quillSubscription = null
498-
}
499-
500501
ngOnChanges(changes: SimpleChanges): void {
501502
if (!this.quillEditor) {
502503
return
@@ -678,9 +679,9 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce
678679
// `AsyncAction` there w/o triggering change detections. We still re-enter the Angular's zone through
679680
// `zone.run` when we emit an event to the parent component.
680681
this.zone.runOutsideAngular(() => {
681-
this.subscription = new Subscription()
682+
this.eventsSubscription = new Subscription()
682683

683-
this.subscription.add(
684+
this.eventsSubscription.add(
684685
// mark model as touched if editor lost focus
685686
fromEvent(this.quillEditor, 'selection-change').subscribe(
686687
([range, oldRange, source]) => {
@@ -699,14 +700,14 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce
699700
editorChange$ = editorChange$.pipe(debounceTime(this.debounceTime()))
700701
}
701702

702-
this.subscription.add(
703+
this.eventsSubscription.add(
703704
// update model if text changes
704705
textChange$.subscribe(([delta, oldDelta, source]) => {
705706
this.textChangeHandler(delta as any, oldDelta as any, source)
706707
})
707708
)
708709

709-
this.subscription.add(
710+
this.eventsSubscription.add(
710711
// triggered if selection or text changed
711712
editorChange$.subscribe(([event, current, old, source]) => {
712713
this.editorChangeHandler(event as 'text-change' | 'selection-change', current, old, source)
@@ -716,10 +717,8 @@ export abstract class QuillEditorBase implements AfterViewInit, ControlValueAcce
716717
}
717718

718719
private dispose(): void {
719-
if (this.subscription !== null) {
720-
this.subscription.unsubscribe()
721-
this.subscription = null
722-
}
720+
this.eventsSubscription?.unsubscribe()
721+
this.eventsSubscription = null
723722
}
724723

725724
private isEmptyValue(html: string | null) {

projects/ngx-quill/src/lib/quill-view-html.component.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
OnChanges,
77
SimpleChanges,
88
ViewEncapsulation,
9+
inject,
910
input,
1011
signal
1112
} from '@angular/core'
@@ -33,10 +34,8 @@ export class QuillViewHTMLComponent implements OnChanges {
3334
readonly innerHTML = signal<SafeHtml>('')
3435
readonly themeClass = signal('ql-snow')
3536

36-
constructor(
37-
private sanitizer: DomSanitizer,
38-
protected service: QuillService
39-
) {}
37+
private sanitizer = inject(DomSanitizer)
38+
private service = inject(QuillService)
4039

4140
ngOnChanges(changes: SimpleChanges) {
4241
if (changes.theme) {

projects/ngx-quill/src/lib/quill-view.component.ts

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,8 @@ import {
77
DestroyRef,
88
ElementRef,
99
EventEmitter,
10-
Inject,
1110
NgZone,
1211
OnChanges,
13-
OnDestroy,
1412
Output,
1513
PLATFORM_ID,
1614
Renderer2,
@@ -22,7 +20,6 @@ import {
2220
} from '@angular/core'
2321
import { takeUntilDestroyed } from '@angular/core/rxjs-interop'
2422
import { DomSanitizer } from '@angular/platform-browser'
25-
import type { Subscription } from 'rxjs'
2623
import { mergeMap } from 'rxjs/operators'
2724

2825
import { CustomModule, CustomOption, QuillBeforeRender, QuillModules } from 'ngx-quill/config'
@@ -42,7 +39,7 @@ import { QuillService } from './quill.service'
4239
<div quill-view-element></div>
4340
`,
4441
})
45-
export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy {
42+
export class QuillViewComponent implements AfterViewInit, OnChanges {
4643
readonly format = input<'object' | 'html' | 'text' | 'json' | undefined>(
4744
undefined
4845
)
@@ -62,19 +59,14 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy {
6259
quillEditor!: QuillType
6360
editorElem!: HTMLElement
6461

65-
private quillSubscription: Subscription | null = null
66-
62+
private elementRef = inject(ElementRef)
63+
private renderer = inject(Renderer2)
64+
private ngZone = inject(NgZone)
65+
private service = inject(QuillService)
66+
private sanitizer = inject(DomSanitizer)
67+
private platformId = inject(PLATFORM_ID)
6768
private destroyRef = inject(DestroyRef)
6869

69-
constructor(
70-
public elementRef: ElementRef,
71-
protected renderer: Renderer2,
72-
protected zone: NgZone,
73-
protected service: QuillService,
74-
protected domSanitizer: DomSanitizer,
75-
@Inject(PLATFORM_ID) protected platformId: any,
76-
) { }
77-
7870
valueSetter = (quillEditor: QuillType, value: any): any => {
7971
const format = getFormat(this.format(), this.service.config.format)
8072
let content = value
@@ -84,7 +76,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy {
8476
if (format === 'html') {
8577
const sanitize = [true, false].includes(this.sanitize()) ? this.sanitize() : (this.service.config.sanitize || false)
8678
if (sanitize) {
87-
value = this.domSanitizer.sanitize(SecurityContext.HTML, value)
79+
value = this.sanitizer.sanitize(SecurityContext.HTML, value)
8880
}
8981
content = quillEditor.clipboard.convert({ html: value })
9082
} else if (format === 'json') {
@@ -112,7 +104,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy {
112104
return
113105
}
114106

115-
this.quillSubscription = this.service.getQuill().pipe(
107+
const quillSubscription = this.service.getQuill().pipe(
116108
mergeMap((Quill) => this.service.beforeRender(Quill, this.customModules(), this.beforeRender()))
117109
).subscribe(Quill => {
118110
const modules = Object.assign({}, this.modules() || this.service.config.modules)
@@ -130,7 +122,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy {
130122
}
131123

132124
let formats = this.formats()
133-
if (!formats && formats === undefined) {
125+
if (formats === undefined) {
134126
formats = this.service.config.formats ? [...this.service.config.formats] : (this.service.config.formats === null ? null : undefined)
135127
}
136128
const theme = this.theme() || (this.service.config.theme ? this.service.config.theme : 'snow')
@@ -139,7 +131,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy {
139131
'[quill-view-element]'
140132
) as HTMLElement
141133

142-
this.zone.runOutsideAngular(() => {
134+
this.ngZone.runOutsideAngular(() => {
143135
this.quillEditor = new Quill(this.editorElem, {
144136
debug,
145137
formats,
@@ -169,10 +161,7 @@ export class QuillViewComponent implements AfterViewInit, OnChanges, OnDestroy {
169161
this.onEditorCreated.emit(this.quillEditor)
170162
})
171163
})
172-
}
173164

174-
ngOnDestroy(): void {
175-
this.quillSubscription?.unsubscribe()
176-
this.quillSubscription = null
165+
this.destroyRef.onDestroy(() => quillSubscription.unsubscribe())
177166
}
178167
}

projects/ngx-quill/src/lib/quill.service.ts

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { DOCUMENT } from '@angular/common'
21
import { inject, Injectable } from '@angular/core'
3-
import { defer, firstValueFrom, forkJoin, from, isObservable, Observable, of } from 'rxjs'
2+
import { defer, forkJoin, isObservable, Observable, of } from 'rxjs'
43
import { map, shareReplay, tap } from 'rxjs/operators'
54

65
import {
@@ -16,30 +15,33 @@ import {
1615
export class QuillService {
1716
readonly config = inject(QUILL_CONFIG_TOKEN) || { modules:defaultModules } as QuillConfig
1817

19-
private document = inject(DOCUMENT)
20-
2118
private Quill!: any
2219

2320
private quill$: Observable<any> = defer(async () => {
2421
if (!this.Quill) {
25-
// Quill adds events listeners on import https://github.com/quilljs/quill/blob/develop/core/emitter.js#L8
26-
// We'd want to use the unpatched `addEventListener` method to have all event callbacks to be run outside of zone.
27-
// We don't know yet if the `zone.js` is used or not, just save the value to restore it back further.
28-
const maybePatchedAddEventListener = this.document.addEventListener
29-
// There're 2 types of Angular applications:
30-
// 1) zone-full (by default)
31-
// 2) zone-less
32-
// The developer can avoid importing the `zone.js` package and tells Angular that he/she is responsible for running
33-
// the change detection by himself. This is done by "nooping" the zone through `CompilerOptions` when bootstrapping
34-
// the root module. We fallback to `document.addEventListener` if `__zone_symbol__addEventListener` is not defined,
35-
// this means the `zone.js` is not imported.
36-
// The `__zone_symbol__addEventListener` is basically a native DOM API, which is not patched by zone.js, thus not even going
37-
// through the `zone.js` task lifecycle. You can also access the native DOM API as follows `target[Zone.__symbol__('methodName')]`.
38-
this.document.addEventListener =
39-
this.document['__zone_symbol__addEventListener'] ||
40-
this.document.addEventListener
22+
// Quill adds event listeners on import:
23+
// https://github.com/quilljs/quill/blob/develop/core/emitter.js#L8
24+
// We want to use the unpatched `addEventListener` method to ensure all event
25+
// callbacks run outside of zone.
26+
// Since we don't yet know whether `zone.js` is used, we simply save the value
27+
// to restore it later.
28+
// We can use global `document` because we execute it only in the browser.
29+
const maybePatchedAddEventListener = document.addEventListener
30+
// There are two types of Angular applications:
31+
// 1) default (with zone.js)
32+
// 2) zoneless
33+
// Developers can avoid importing the `zone.js` package and inform Angular that
34+
// they are responsible for running change detection manually.
35+
// This can be done using `provideZonelessChangeDetection()`.
36+
// We fall back to `document.addEventListener` if `__zone_symbol__addEventListener`
37+
// is not defined, which indicates that `zone.js` is not imported.
38+
// The `__zone_symbol__addEventListener` is essentially the native DOM API,
39+
// unpatched by zone.js, meaning it does not go through the `zone.js` task lifecycle.
40+
document.addEventListener =
41+
document['__zone_symbol__addEventListener'] ||
42+
document.addEventListener
4143
const { Quill } = await import('./quill')
42-
this.document.addEventListener = maybePatchedAddEventListener
44+
document.addEventListener = maybePatchedAddEventListener
4345
this.Quill = Quill
4446
}
4547

@@ -54,11 +56,14 @@ export class QuillService {
5456
)
5557
})
5658

57-
return firstValueFrom(this.registerCustomModules(
58-
this.Quill,
59-
this.config.customModules,
60-
this.config.suppressGlobalRegisterWarning
61-
))
59+
// Use `Promise` directly to avoid bundling `firstValueFrom`.
60+
return new Promise(resolve => {
61+
this.registerCustomModules(
62+
this.Quill,
63+
this.config.customModules,
64+
this.config.suppressGlobalRegisterWarning
65+
).subscribe(resolve)
66+
})
6267
}).pipe(
6368
shareReplay({
6469
bufferSize: 1,
@@ -80,9 +85,9 @@ export class QuillService {
8085
// so it operates individually per component. If no custom module needs to be
8186
// registered and no `beforeRender` function is provided, it will emit
8287
// immediately and proceed with the rendering.
83-
const sources = [this.registerCustomModules(Quill, customModules)]
88+
const sources: (Observable<any> | Promise<any>)[] = [this.registerCustomModules(Quill, customModules)]
8489
if (beforeRender) {
85-
sources.push(from(beforeRender()))
90+
sources.push(beforeRender())
8691
}
8792
return forkJoin(sources).pipe(map(() => Quill))
8893
}

0 commit comments

Comments
 (0)