Skip to content

fix: add disconnectedCallback() lifecycle method to unmount map #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jessicamcinchak merged 3 commits into from
Dec 14, 2021
Merged

fix: add disconnectedCallback() lifecycle method to unmount map #97

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20046ed
add disconnectedCallback() lifecycle method
jessicamcinchak Dec 11, 2021
ca37bdb
add constructor
jessicamcinchak Dec 14, 2021
7c277e5
don't use @state decorator
jessicamcinchak Dec 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/my-map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,17 @@ export class MyMap extends LitElement {
@property({ type: Boolean })
useScaleBarStyle = false;

// set class property (map doesn't require any reactivity using @state)
map?: Map;

// called when element is created
constructor() {
super();
}

// runs after the initial render
firstUpdated() {
const target = this.shadowRoot?.querySelector("#map") as HTMLElement;
const target = this.renderRoot.querySelector("#map") as HTMLElement;

const useVectorTiles =
!this.disableVectorTiles && Boolean(this.osVectorTilesApiKey);
Expand Down Expand Up @@ -204,6 +212,8 @@ export class MyMap extends LitElement {
}),
});

this.map = map;

// make configurable interactions available
const draw = configureDraw(this.drawPointer);
const modify = configureModify(this.drawPointer);
Expand All @@ -230,6 +240,8 @@ export class MyMap extends LitElement {
}
};

// this is an internal event listener, so doesn't need to be removed later
// ref https://lit.dev/docs/components/lifecycle/#disconnectedcallback
Copy link

@peterszerzo peterszerzo Dec 14, 2021

Choose a reason for hiding this comment

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

Are you sure you get away with not having to clean this up if you didn't use their render() method with the html tagged template literal?

Copy link
Member Author

@jessicamcinchak jessicamcinchak Dec 14, 2021

Choose a reason for hiding this comment

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

mostly sure 🙃 the button & it's attached listener seem to be fully encapsulated in the map's shadow DOM on inspection, and the docs say "By default, a bubbling custom event fired inside shadow DOM will stop bubbling when it reaches the shadow root." so I think that means it can be considered "internal" & shouldn't prevent the map from unmounting?

https://lit.dev/docs/v1/components/events/#add-event-listeners-after-first-paint

Copy link

@peterszerzo peterszerzo Dec 14, 2021

Choose a reason for hiding this comment

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

Oh right, that makes sense!

button.addEventListener("click", handleReset, false);

const element = document.createElement("div");
Expand Down Expand Up @@ -435,6 +447,12 @@ export class MyMap extends LitElement {
<div id="map" tabindex="0" />`;
}

// unmount the map
disconnectedCallback() {
super.disconnectedCallback();
this.map?.dispose();
}

/**
* dispatches an event for clients to subscribe to
* @param eventName
Expand Down