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.

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

Jump to bottom

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

@caalador
fix: Remove removed items from array
4e8a2c1 
Remove any elements added to the
global array when the DOMNode is removed
from the dom.

Closes #11608
@caalador caalador added this to Iteration Reviews in OLD Vaadin Flow ongoing work (Vaadin 10+) via automation Aug 25, 2021
pleku
pleku suggested changes Aug 25, 2021
View reviewed changes
flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js Outdated Show resolved Hide resolved
flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js Outdated
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

@caalador
Only keep track of css injected to document.
318145c 
Do not push multiples of the same CSS to the
document.
@pleku pleku requested a review from tomivirkki August 26, 2021 11:30
@pleku pleku merged commit 8da204a into master Aug 27, 2021
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Iteration Reviews to Done - pending release 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
@caalador @vaadin-bot
fix: keep track of css injected to document (#11635)
d9c2670 
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
@caalador @vaadin-bot
fix: keep track of css injected to document (#11635)
e155199 
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
@vaadin-bot @caalador
fix: keep track of css injected to document (#11635) (#11658)
f2bd756 
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608

Co-authored-by: caalador <mikael.grankvist@vaadin.com>
vaadin-bot added a commit that referenced this pull request Aug 27, 2021
@vaadin-bot @caalador
fix: keep track of css injected to document (#11635) (#11657)
316d993 
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Closes #11608

Co-authored-by: caalador <mikael.grankvist@vaadin.com>
Artur- added a commit that referenced this pull request Aug 27, 2021
@Artur-
Revert "fix: keep track of css injected to document (#11635)"
c8e010d 
This reverts commit 8da204a.
@Artur- Artur- mentioned this pull request Aug 27, 2021
Closed
pleku added a commit that referenced this pull request Aug 27, 2021
@pleku
fix: not fail for missing window.Vaadin.Flow
9deca8a 
Fixes regression caused by #11635 
Fixes #11666
@pleku pleku mentioned this pull request Aug 27, 2021
Merged
pleku added a commit that referenced this pull request Aug 27, 2021
@pleku
fix: not fail for missing window.Vaadin.Flow (#11669)
9cc240b 
Fixes regression caused by #11635 
Fixes #11666
vaadin-bot pushed a commit that referenced this pull request Aug 27, 2021
@pleku @vaadin-bot
fix: not fail for missing window.Vaadin.Flow (#11669)
4aba3b5 
Fixes regression caused by #11635 
Fixes #11666
vaadin-bot added a commit that referenced this pull request Aug 28, 2021
@vaadin-bot @pleku
fix: not fail for missing window.Vaadin.Flow (#11669) (#11670)
2d600d9 
Fixes regression caused by #11635 
Fixes #11666

Co-authored-by: Pekka Hyvönen <pekka@vaadin.com>
mshabarov pushed a commit that referenced this pull request Aug 29, 2021
@pleku @mshabarov
fix: not fail for missing window.Vaadin.Flow (#11669)
caebcb7 
Fixes regression caused by #11635
Fixes #11666

(cherry picked from commit 9cc240b)
@mshabarov mshabarov mentioned this pull request Aug 29, 2021
Merged
9 tasks
caalador pushed a commit that referenced this pull request Aug 30, 2021
@mshabarov @pleku
fix: not fail for missing window.Vaadin.Flow (#11669) (#11672)
6135e21 
Fixes regression caused by #11635
Fixes #11666

(cherry picked from commit 9cc240b)

Co-authored-by: Pekka Hyvönen <pekka@vaadin.com>
caalador added a commit that referenced this pull request Aug 30, 2021
@caalador
fix: keep track of css injected to document (#11635)
2e2ad01 
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
@caalador
fix: keep track of css injected to document (#11635)
681f821 
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
@caalador
fix: keep track of css injected to document (#11635)
5451d06 
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
@caalador
fix: keep track of css injected to document (#11635) (#11673)
204d2cf 
Stops css/fonts from being loaded multiple times to the document with embedded applications.

Fixes #11608
@vaadin-bot vaadin-bot mentioned this pull request Sep 7, 2021
Merged
caalador added a commit that referenced this pull request Sep 8, 2021
@caalador
fix: keep track of css injected to document (#11635) (#11673)
fc31f9d 
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
@caalador
fix: keep track of css injected to document (#11635) (#11673) (#11793)
99a1a54 
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
Reviewers

@tomivirkki tomivirkki tomivirkki approved these changes

@pleku pleku pleku approved these changes

Assignees
No one assigned
Labels
cherry-picked-7.0 cherry-picked-8.0 need to pick manually 2.6 need to pick manually 2.7 +0.0.1
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Memory leak in theme generator
4 participants
@caalador @vaadin-bot @tomivirkki @pleku