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(backend): happy-domで外部HTMLをパースする際に関連リソースが読み込まれる問題を修正 #14521

Merged

Conversation

kakkokari-gtyih
Copy link
Contributor

@kakkokari-gtyih kakkokari-gtyih commented Sep 6, 2024

version 10 didn't quite support disabling all of that

I have tested that MfmService (the other code that uses happy-dom) still works fine: the RSS feed for a user is generated correctly, with HTML rendered from MFM

(cherry picked from commit 26e0412fbb91447c37e8fb06ffb0487346063bb8)

What

  • happy-domをv15にアップデート
  • happy-domで外部HTMLをパースする際に関連リソースが読み込まれるのを修正(この修正はv10では対応不可だったらしい)

https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/610?commit_id=f244d425005ae662cfb94f2a18e38cedadfb3c0a#note_5069

Why

無駄かも

Additional info (optional)

v14で見られたメモリリークはv15になるときのリファクタリングで解消されたかもしれないという報告があるけど真偽は不明

capricorn86/happy-dom#1055 (comment)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

dakkar and others added 2 commits September 7, 2024 02:58
version 10 didn't quite support disabling all of that

I have tested that `MfmService` (the other code that uses `happy-dom`)
still works fine: the RSS feed for a user is generated correctly, with
HTML rendered from MFM

(cherry picked from commit 26e0412)
@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Sep 6, 2024
Copy link
Contributor

github-actions bot commented Sep 6, 2024

このPRによるapi.jsonの差分

差分はこちら

Get diff files from Workflow Page

Copy link

codecov bot commented Sep 6, 2024

Codecov Report

Attention: Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.

Project coverage is 39.62%. Comparing base (837a8e1) to head (f9c7bd1).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
...s/backend/src/core/activitypub/ApRequestService.ts 0.00% 33 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #14521       +/-   ##
============================================
+ Coverage    19.53%   39.62%   +20.09%     
============================================
  Files          713     1535      +822     
  Lines       100287   190816    +90529     
  Branches       997     2597     +1600     
============================================
+ Hits         19591    75618    +56027     
- Misses       80152   114637    +34485     
- Partials       544      561       +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@syuilo syuilo merged commit be0906a into misskey-dev:develop Sep 15, 2024
28 of 30 checks passed
@syuilo
Copy link
Member

syuilo commented Sep 15, 2024

🙏🏿

@kakkokari-gtyih kakkokari-gtyih deleted the fix-avoid-load-external-resources branch September 15, 2024 03:34
@kakkokari-gtyih
Copy link
Contributor Author

Sharkeyでこの変更からしばらくしたあとにメモリ使用量の瞬間的な増加が見られたらしいのでやっぱあかんかもしれない(原因の切り分けまではまだ行われていないので今なにかをするというよりかはそういうことがあったということを覚えておく程度で良い)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants