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: node.js の punycode モジュールが使用されている場所がある問題 #15248

Merged
merged 14 commits into from
Jan 14, 2025

Conversation

anatawa12
Copy link
Member

@anatawa12 anatawa12 commented Jan 10, 2025

What

Fixing #15246 but not closing since some dependency still uses

ついでに今後の事故をなくすため lint を追加しました。

Why

Warningがうるさい。
多分モジュールがnodejsから削除されたら静になるけどまだそうならないので

Additional info (optional)

CHANGELOGなくても良い機がする

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

@github-actions github-actions bot added packages/frontend Client side specific issue/PR packages/backend Server side specific issue/PR labels Jan 10, 2025
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 70.83333% with 7 lines in your changes missing coverage. Please review.

Project coverage is 39.39%. Comparing base (319f7e6) to head (f2b3053).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
packages/backend/src/core/AiService.ts 33.33% 2 Missing ⚠️
packages/backend/src/core/UtilityService.ts 66.66% 1 Missing ⚠️
...kages/backend/src/server/api/endpoints/i/update.ts 0.00% 1 Missing ⚠️
...ackages/frontend/src/components/MkSignin.input.vue 0.00% 1 Missing ⚠️
...es/frontend/src/components/MkSignupDialog.form.vue 0.00% 1 Missing ⚠️
packages/frontend/src/pages/theme-editor.vue 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #15248      +/-   ##
===========================================
- Coverage    41.12%   39.39%   -1.74%     
===========================================
  Files         1572     1568       -4     
  Lines       204875   199065    -5810     
  Branches      3679     3646      -33     
===========================================
- Hits         84259    78416    -5843     
- Misses      120009   120075      +66     
+ Partials       607      574      -33     

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

@samunohito
Copy link
Member

SyntaxError: The requested module 'punycode/' does not provide an export named 'toASCII'

おお・・・?

@anatawa12
Copy link
Member Author

/で終わってる場合、mainを読む仕組みだったけどindex.jsを読むのが無効化されてるからだめとかかな。いい感じにパスにします。

@anatawa12
Copy link
Member Author

anatawa12 commented Jan 10, 2025

importすべきはesmoduleなpunycode.es6.jsっぽいけどpunycode.es6.js@types/punycodeにない...

@anatawa12
Copy link
Member Author

anatawa12 commented Jan 10, 2025

commonjsの方だとエラーになったので、 'punycode/punycode.es6.js' のd.tsを生やして対処しました。 punycode/punycode.es6.jsがesmoduleじゃないからimportできなかった

Copy link
Contributor

github-actions bot commented Jan 10, 2025

このPRによるapi.jsonの差分
差分はありません。
Get diff files from Workflow Page

@anatawa12
Copy link
Member Author

mathiasbynens/punycode.js#142 とりあえず .mjs を作る PR 投げたけど、 backend で punycode 使ってるの utility service だけなので、別のAPI叩くようにしたほうが良さそう。

@anatawa12 anatawa12 changed the title fix: punycode.js が使用されていない場所がある問題 fix: node.js の punycode モジュールが使用されている場所がある問題 Jan 10, 2025
@anatawa12
Copy link
Member Author

backendではpunycode.jsを使わずnode:urlを使うことにしました

@anatawa12
Copy link
Member Author

どうにかchecks通った(chromaticは原因なんだろ)

@u1-liquid
Copy link
Contributor

結局のところ、当方で使用している punycode パッケージを変更しても、どこかで参照されているため警告メッセージは消えません。

<checksのログから>
(node:3575) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.

また、import 文で/を付けるトリックを使うよりも、punycode.jsパッケージ(punycodeパッケージと同時に公開されています)を使用したほうがよろしいのではないかと思います。
https://github.com/MisskeyIO/misskey/pull/833/files#diff-8571ecd91584b00015b23695d3a6a164282636bb47bfbe46dca243bf9b4db773R61
https://github.com/MisskeyIO/misskey/pull/833/files#diff-8571ecd91584b00015b23695d3a6a164282636bb47bfbe46dca243bf9b4db773R109

@anatawa12
Copy link
Member Author

anatawa12 commented Jan 10, 2025

ありがとうございます。punycode.jsを使用するようにしました。

warningが残る問題については依存関係の問題ですが、grepした限りだと [email protected] [email protected] [email protected] [email protected] [email protected] が使用していたため、これらの関連パッケージの方を調査しますが、とりあえずmisskeyから使用をなくすことに価値はあると思います

 pnpm list --depth Infinity [email protected] [email protected] [email protected] [email protected] [email protected]
@nestjs/core 10.4.7
└─┬ @nuxtjs/opencollective 0.3.2
  └─┬ node-fetch 2.7.0
    └─┬ whatwg-url 5.0.0
      └── tr46 0.0.3
@nestjs/testing 10.4.7
└─┬ @nestjs/core 10.4.7 peer
  └─┬ @nuxtjs/opencollective 0.3.2
    └─┬ node-fetch 2.7.0
      └─┬ whatwg-url 5.0.0
        └── tr46 0.0.3
@simplewebauthn/server 10.0.1
└─┬ cross-fetch 4.0.0
  └─┬ node-fetch 2.7.0
    └─┬ whatwg-url 5.0.0
      └── tr46 0.0.3
jsdom 24.1.1
└─┬ tough-cookie 4.1.4
  └── psl 1.9.0
nsfwjs 2.4.2
├─┬ @nsfw-filter/gif-frames 1.0.2
│ └─┬ get-pixels-frame-info-update 3.3.2
│   └─┬ request 2.88.2
│     ├─┬ har-validator 5.1.5
│     │ └─┬ ajv 6.12.6
│     │   └── uri-js 4.4.1
│     └─┬ tough-cookie 2.5.0
│       └── psl 1.9.0
└─┬ @tensorflow/tfjs 4.4.0 peer
  ├─┬ @tensorflow/tfjs-backend-cpu 4.4.0
  │ └─┬ @tensorflow/tfjs-core 4.4.0 peer
  │   └─┬ node-fetch 2.6.13
  │     └─┬ whatwg-url 5.0.0
  │       └── tr46 0.0.3
  ├─┬ @tensorflow/tfjs-backend-webgl 4.4.0
  │ ├─┬ @tensorflow/tfjs-backend-cpu 4.4.0
  │ │ └─┬ @tensorflow/tfjs-core 4.4.0 peer
  │ │   └─┬ node-fetch 2.6.13
  │ │     └─┬ whatwg-url 5.0.0
  │ │       └── tr46 0.0.3
  │ └─┬ @tensorflow/tfjs-core 4.4.0 peer
  │   └─┬ node-fetch 2.6.13
  │     └─┬ whatwg-url 5.0.0
  │       └── tr46 0.0.3
  └─┬ @tensorflow/tfjs-converter 4.4.0
    └─┬ @tensorflow/tfjs-core 4.4.0 peer
      └─┬ node-fetch 2.6.13
        └─┬ whatwg-url 5.0.0
          └── tr46 0.0.3

以下調査結果

pslについては更新すれば良いかもしれない([email protected]で修正を試みている模様)

@anatawa12
Copy link
Member Author

依存関係をいい感じにして改善しました。
まだ完全な修正になってないです。

  • @nuxtjs/opencollective関連についてはランタイムには実行されないので無視しました。
  • @simplewebauthn/serverに関しては upstream の対応待ちになると思います。
  • @nsfw-filter/gif-framesに関してはnsfwjsを更新することで対処しました。
    • 何故か a5e0705 の変更によってDOMWindow['document']が解決できなくなったのでDocument43bf8ec で書き換えました
  • @tensorflow/tfjs-core 周りに関しては node-fetch@3fetchtf.env().global.fetch に与えることで対処しました
    • これにより tensorflow によって node-fetch@2require() されることがなくなります。(もともと require されることなかったかもしれないですが)

@mi-gh-maintainer mi-gh-maintainer bot merged commit 145c6cf into misskey-dev:develop Jan 14, 2025
12 of 18 checks passed
Copy link

Thank you 🙏

Ruruke pushed a commit to Ruruke/misskey that referenced this pull request Jan 14, 2025
* fix: punycode.js が使用されていない場所がある問題

* fix: use punycode/punycode.js on backend

* fix: use punycode/punycode.es6.js on backend

* fix: d.ts missing declare keyword

* chore: don't use punycode.js on backend

* update pnpm-lock.yaml

* chore: remove punycode.d.ts

* chore: use punycode.js instead of punycode npm package

* chore: bump psl to 1.15.0

* chore: bump nsfwjs to 4.2.0

4.2.1 is not usable because of infinitered/nsfwjs#904

* chore: prevent loading node-fetch from tensorflow

* chore: DOMWindow['document'] => Document

IDK why DOMWindow['document'] fails, but might be related to tsc internal complexity limit

* fix: disable --trace-deprecation

---------

Co-authored-by: syuilo <[email protected]>
@anatawa12 anatawa12 mentioned this pull request Jan 24, 2025
5 tasks
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 packages/frontend Client side specific issue/PR
Projects
Development

Successfully merging this pull request may close these issues.

4 participants