Skip to content
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.

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

fix: Remove removed items from array #11635

Merged
merged 5 commits into from
Aug 27, 2021
Merged

fix: Remove removed items from array #11635

merged 5 commits into from
Aug 27, 2021

Conversation

caalador
Copy link
Contributor

Remove any elements added to the
global array when the DOMNode is removed
from the dom.

Closes #11608

Remove any elements added to the
global array when the DOMNode is removed
from the dom.

Closes #11608
eventTarget.addEventListener('DOMNodeRemoved', event => {
const index = window.Vaadin['${globalCssFlag}'].indexOf(target);
window.Vaadin['${globalCssFlag}'].splice(index, 1);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

If/when using an observer instead, it can be disconnected after the clean-up

Do not push multiples of the same CSS to the
document.
@caalador caalador force-pushed the issues/11608_mem_leak branch from 1c0c745 to 318145c Compare August 26, 2021 07:31
@pleku pleku requested a review from tomivirkki August 26, 2021 11:30
@pleku pleku merged commit 8da204a into master Aug 27, 2021
@pleku pleku deleted the issues/11608_mem_leak branch August 27, 2021 12:42
@pleku pleku added +0.0.1 and removed +0.1.0 labels Aug 27, 2021
vaadin-bot pushed a commit that referenced this pull request Aug 27, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608
@vaadin-bot
Copy link
Collaborator

Hi @caalador and @pleku, when i performed cherry-pick to this commit to 2.7, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 8da204a
error: could not apply 8da204a... fix: keep track of css injected to document (#11635)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

vaadin-bot pushed a commit that referenced this pull request Aug 27, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608
@vaadin-bot
Copy link
Collaborator

Hi @caalador and @pleku, when i performed cherry-pick to this commit to 2.6, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 8da204a
error: could not apply 8da204a... fix: keep track of css injected to document (#11635)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

vaadin-bot added a commit that referenced this pull request Aug 27, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608

Co-authored-by: caalador <[email protected]>
vaadin-bot added a commit that referenced this pull request Aug 27, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608

Co-authored-by: caalador <[email protected]>
Artur- added a commit that referenced this pull request Aug 27, 2021
pleku added a commit that referenced this pull request Aug 27, 2021
pleku added a commit that referenced this pull request Aug 27, 2021
vaadin-bot pushed a commit that referenced this pull request Aug 27, 2021
vaadin-bot added a commit that referenced this pull request Aug 28, 2021
Fixes regression caused by #11635 
Fixes #11666

Co-authored-by: Pekka Hyvönen <[email protected]>
mshabarov pushed a commit that referenced this pull request Aug 29, 2021
Fixes regression caused by #11635
Fixes #11666

(cherry picked from commit 9cc240b)
caalador pushed a commit that referenced this pull request Aug 30, 2021
Fixes regression caused by #11635
Fixes #11666

(cherry picked from commit 9cc240b)

Co-authored-by: Pekka Hyvönen <[email protected]>
caalador added a commit that referenced this pull request Aug 30, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608
# Conflicts:
#	flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js
caalador added a commit that referenced this pull request Aug 31, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608
# Conflicts:
#	flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js
caalador added a commit that referenced this pull request Sep 1, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608
# Conflicts:
#	flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js
mshabarov pushed a commit that referenced this pull request Sep 7, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Fixes #11608
caalador added a commit that referenced this pull request Sep 8, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Fixes #11608
# Conflicts:
#	flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdateImports.java
mshabarov pushed a commit that referenced this pull request Sep 9, 2021
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Fixes #11608
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.

Memory leak in theme generator
4 participants