Skip to content

Commit

Permalink
feat(platform-browser): enable removal of styles on component destroy…
Browse files Browse the repository at this point in the history 
… by default (#51571)

This change aligns the settings between G3 and P3 as `REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT` is already set to `true` internally.

BREAKING CHANGE: `REMOVE_STYLES_ON_COMPONENT_DESTROY` default value is now `true`. This causes CSS of components to be removed from the DOM when destroyed. You retain the previous behaviour by providing the `REMOVE_STYLES_ON_COMPONENT_DESTROY` injection token.

```ts
import {REMOVE_STYLES_ON_COMPONENT_DESTROY} from '@angular/platform-browser';
...
providers: [{
  provide: REMOVE_STYLES_ON_COMPONENT_DESTROY,
  useValue: false,
}]
```

PR Close #51571
  • Loading branch information
@alan-agius4 @thePunderWoman
alan-agius4 authored and thePunderWoman committed Aug 30, 2023
1 parent 0f86a0b commit c340d6e
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 56 deletions.
4 changes: 2 additions & 2 deletions packages/platform-browser/src/dom/dom_renderer.ts
View file
Expand Up @@ -32,13 +32,13 @@ export const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
/**
* The default value for the `REMOVE_STYLES_ON_COMPONENT_DESTROY` DI token.
*/
const REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT = false;
const REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT = true;

/**
* A [DI token](guide/glossary#di-token "DI token definition") that indicates whether styles
* of destroyed components should be removed from DOM.
*
* By default, the value is set to `false`. This will be changed in the next major version.
* By default, the value is set to `true`.
* @publicApi
*/
export const REMOVE_STYLES_ON_COMPONENT_DESTROY =
Expand Down
108 changes: 54 additions & 54 deletions packages/platform-browser/test/dom/dom_renderer_spec.ts
View file
Expand Up @@ -143,56 +143,8 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
expect(otherChild.parentNode).toBe(template.content);
});

describe('should not cleanup styles of destroyed components by default', () => {
it('works for components without encapsulation emulated', async () => {
const fixture = TestBed.createComponent(SomeAppForCleanUp);
const compInstance = fixture.componentInstance;
compInstance.showEmulatedComponents = true;

fixture.detectChanges();
// verify style is in DOM
expect(await styleCount(fixture, '.emulated')).toBe(1);

// Remove a single instance of the component.
compInstance.componentOneInstanceHidden = true;
fixture.detectChanges();
// Verify style is still in DOM
expect(await styleCount(fixture, '.emulated')).toBe(1);

// Hide all instances of the component
compInstance.componentTwoInstanceHidden = true;
fixture.detectChanges();

// Verify style is still in DOM
expect(await styleCount(fixture, '.emulated')).toBe(1);
});

it('works for components without encapsulation none', async () => {
const fixture = TestBed.createComponent(SomeAppForCleanUp);
const compInstance = fixture.componentInstance;
compInstance.showEmulatedComponents = false;

fixture.detectChanges();
// verify style is in DOM
expect(await styleCount(fixture, '.none')).toBe(1);

// Remove a single instance of the component.
compInstance.componentOneInstanceHidden = true;
fixture.detectChanges();
// Verify style is still in DOM
expect(await styleCount(fixture, '.none')).toBe(1);

// Hide all instances of the component
compInstance.componentTwoInstanceHidden = true;
fixture.detectChanges();

// Verify style is still in DOM
expect(await styleCount(fixture, '.none')).toBe(1);
});
});

describe(
'should cleanup styles of destroyed components when `REMOVE_STYLES_ON_COMPONENT_DESTROY` is `true`',
'should not cleanup styles of destroyed components when `REMOVE_STYLES_ON_COMPONENT_DESTROY` is `false`',
() => {
beforeEach(() => {
TestBed.resetTestingModule();
Expand All @@ -206,7 +158,7 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
providers: [
{
provide: REMOVE_STYLES_ON_COMPONENT_DESTROY,
useValue: true,
useValue: false,
},
],
});
Expand All @@ -216,6 +168,7 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
const fixture = TestBed.createComponent(SomeAppForCleanUp);
const compInstance = fixture.componentInstance;
compInstance.showEmulatedComponents = true;

fixture.detectChanges();
// verify style is in DOM
expect(await styleCount(fixture, '.emulated')).toBe(1);
Expand All @@ -230,8 +183,8 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
compInstance.componentTwoInstanceHidden = true;
fixture.detectChanges();

// Verify style is not in DOM
expect(await styleCount(fixture, '.emulated')).toBe(0);
// Verify style is still in DOM
expect(await styleCount(fixture, '.emulated')).toBe(1);
});

it('works for components without encapsulation none', async () => {
Expand All @@ -253,10 +206,57 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
compInstance.componentTwoInstanceHidden = true;
fixture.detectChanges();

// Verify style is not in DOM
expect(await styleCount(fixture, '.emulated')).toBe(0);
// Verify style is still in DOM
expect(await styleCount(fixture, '.none')).toBe(1);
});
});

describe('should cleanup styles of destroyed components by default', () => {
it('works for components without encapsulation emulated', async () => {
const fixture = TestBed.createComponent(SomeAppForCleanUp);
const compInstance = fixture.componentInstance;
compInstance.showEmulatedComponents = true;
fixture.detectChanges();
// verify style is in DOM
expect(await styleCount(fixture, '.emulated')).toBe(1);

// Remove a single instance of the component.
compInstance.componentOneInstanceHidden = true;
fixture.detectChanges();
// Verify style is still in DOM
expect(await styleCount(fixture, '.emulated')).toBe(1);

// Hide all instances of the component
compInstance.componentTwoInstanceHidden = true;
fixture.detectChanges();

// Verify style is not in DOM
expect(await styleCount(fixture, '.emulated')).toBe(0);
});

it('works for components without encapsulation none', async () => {
const fixture = TestBed.createComponent(SomeAppForCleanUp);
const compInstance = fixture.componentInstance;
compInstance.showEmulatedComponents = false;

fixture.detectChanges();
// verify style is in DOM
expect(await styleCount(fixture, '.none')).toBe(1);

// Remove a single instance of the component.
compInstance.componentOneInstanceHidden = true;
fixture.detectChanges();
// Verify style is still in DOM
expect(await styleCount(fixture, '.none')).toBe(1);

// Hide all instances of the component
compInstance.componentTwoInstanceHidden = true;
fixture.detectChanges();

// Verify style is not in DOM
expect(await styleCount(fixture, '.emulated')).toBe(0);
});
});
});
}

Expand Down

0 comments on commit c340d6e

Please sign in to comment.