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

非画像ファイルがアップロードされた時リンクで表示されるようにした #5659

Merged
merged 3 commits into from
Nov 21, 2022

Conversation

keiz1213
Copy link
Contributor

@keiz1213 keiz1213 commented Oct 18, 2022

issue

概要

pdfファイル等の非画像ファイル(png、jpeg、gif以外)をアップロードした時に表示が崩れていました。
これを表示が崩れないようにリンク(aタグ)として表示するようにしました。
Image from Gyazo

備考

  • bootcampではサイト内テキストエリアをtextarea-markdownというパッケージを使って拡張しています。今回こちらのパッケージにPRをおくりマージしていただきました。
    修正前は、アップロードされる際に生成されるマークダウンテキストがすべてimgタグ用(![fileName](url))でした。それを、アップロードされる画像の種類に応じてimgタグ用とaタグ用([fileName](url))に振り分けるようにしました。
    rendering non image file into a link with file size komagata/textarea-markdown#31

  • ファイルの種類を判別するためにMagic Bytesというパッケージを使用しています

  • ファイルのサイズをわかりやすく表示するためにfilesize.jsというパッケージを使用しています

  • 画像ファイルはpngjpeggifを想定しています。

変更前

Image from Gyazo

変更後

Image from Gyazo

変更確認方法

  1. feature/rendering-non-image-file-into-a-linkをローカルに取り込む
  2. bin/setupを実行する
  3. rails sでサーバーを起動する
  4. 任意のユーザーでログインする
  5. 日報作成画面等で非画像ファイル(txtやpdfなど)をアップロードしリンクで表示されること、ファイルのkb数が合ってることを確認する
  6. 非画像ファイル(txtやpdfなど)から拡張子を削除して再度アップロードしリンクで表示されること、ファイルのkb数が合ってることを確認する(拡張子がないファイルの場合の対応)
  7. 日報作成画面等で画像ファイル(png,jpeg,gif)をアップロードし画像が表示されることを確認する
  8. 画像ファイル名(png,jpeg,gif)から拡張子を削除して再度アップロードし画像が表示されることを確認する(拡張子がないファイルの場合の対応)

@keiz1213 keiz1213 self-assigned this Oct 18, 2022
@keiz1213
Copy link
Contributor Author

keiz1213 commented Nov 4, 2022

@komagata

お疲れ様です。実装方針について1点質問があります。

先日のミーティングでご指摘いただいて、現在実装方針を改めて考え直しています。

前回までの実装方法では、markdown-itの画像に関するレンダリングルールを上書きし、トークンの文字列の中に画像の拡張子が含まれていればデフォルトのレンダラーで出力し、含まれていなければaタグを出力する、という風にしてimgタグとaタグを振り分けていました。

そして今考えているのはtextarea-markdownuploadメソッドに手を加えるという方向性で考えています。
uploadメソッドの中で、fileオブジェクトに画像の拡張子が含まれていれば通常通り画像用のマークダウンテキスト(![fileName](url))を生成し、含まれていなければリンク用のマークダウンテキスト([fileName](url))を生成する、という方向性で考えているのですがこの方向性で進めても大丈夫でしょうか?

https://github.com/komagata/textarea-markdown/blob/8d336d5c92211add622010613088921cbbe8157a/src/textarea-markdown.js#L93

@komagata
Copy link
Member

komagata commented Nov 4, 2022

@keiz1213 基本的にはその方向でOKです〜。

拡張子はついてない場合があるのでこの辺りを使うのはどうでしょうか?
https://www.npmjs.com/package/file-type

@keiz1213
Copy link
Contributor Author

keiz1213 commented Nov 5, 2022

@komagata

回答ありがとうございます!

一応確認なのですが、そうなるとtextarea-markdownのリポジトリにPRを出すという流れで大丈夫でしょうか?

@komagata
Copy link
Member

komagata commented Nov 5, 2022

@keiz1213 はい、そうです〜

@keiz1213
Copy link
Contributor Author

keiz1213 commented Nov 5, 2022

@komagata

わかりました!ありがとうございます!

@keiz1213
Copy link
Contributor Author

@komagata

お疲れさまです。1点質問があります。

file-typeの代わりにmagic-bytes.jsを使っても良いか

教えていただいたfile-typeパッケージをtextarea-markdownにインストールするとwebpackのコンパイル時にエラーが出るようになりました。このエラーの解決方法を探す過程でmagic-bytes.jsというパッケージを見つけ、試しにこのパッケージをtextarea-markdownにインストールして使用したところ、コンパイルも上手くいき、拡張子がないファイルでも画像ファイルか否かを判別できました。magic-bytes.jsではファイルのマジックナンバーでファイルの種類を判別しています。

file-typeのコンパイルエラーを解消してfile-typeを使うか、それともmagic-bytes.jsを使うか、どちらがよろしいでしょうか?個人的にはブラウザで無難に動くmagic-bytes.jsを使いたいと思っています。

以下、コンパイル時のエラー詳細です。

コンパイル時のエラー

file-typeをtextarea-markdownにインストールしてコンパイルするときに発生するエラーです。

❯ npx webpack --mode development                                                   
asset main.js 588 KiB [emitted] (name: main)
asset node_stream.js 2.19 KiB [emitted]
runtime modules 8.06 KiB 11 modules
modules by path ./textarea-markdown/node_modules/ 440 KiB 102 modules
modules with errors 78 bytes [errors]
  node:buffer 39 bytes [built] [code generated] [1 error]
  node:stream 39 bytes [built] [code generated] [1 error]
./src/index.js 324 bytes [built] [code generated]
./textarea-markdown/src/textarea-markdown.js 3.75 KiB [built] [code generated]
./node_modules/punycode/punycode.es6.js 12.3 KiB [built] [code generated]
util (ignored) 15 bytes [built] [code generated]
util (ignored) 15 bytes [built] [code generated]

ERROR in ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib/_stream_readable.js 46:13-37
Module not found: Error: Can't resolve 'buffer' in '/Users/nakamurakeizou/textarea5/textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
        - install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "buffer": false }
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/readable-browser.js 1:10-63
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/lib/index.js 4:26-52
 @ ./textarea-markdown/node_modules/file-type/browser.js 2:0-68 36:37-60
 @ ./textarea-markdown/src/textarea-markdown.js 3:0-44
 @ ./src/index.js 1:0-73 5:6-22

ERROR in ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib/_stream_writable.js 70:13-37
Module not found: Error: Can't resolve 'buffer' in '/Users/nakamurakeizou/textarea5/textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
        - install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "buffer": false }
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/readable-browser.js 4:0-55
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/lib/index.js 4:26-52
 @ ./textarea-markdown/node_modules/file-type/browser.js 2:0-68 36:37-60
 @ ./textarea-markdown/src/textarea-markdown.js 3:0-44
 @ ./src/index.js 1:0-73 5:6-22

ERROR in ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib/internal/streams/buffer_list.js 15:15-32
Module not found: Error: Can't resolve 'buffer' in '/Users/nakamurakeizou/textarea5/textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib/internal/streams'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
        - install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "buffer": false }
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib/_stream_readable.js 72:17-58
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/readable-browser.js 1:10-63
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/lib/index.js 4:26-52
 @ ./textarea-markdown/node_modules/file-type/browser.js 2:0-68 36:37-60
 @ ./textarea-markdown/src/textarea-markdown.js 3:0-44
 @ ./src/index.js 1:0-73 5:6-22

ERROR in ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/safe-buffer/index.js 3:13-30
Module not found: Error: Can't resolve 'buffer' in '/Users/nakamurakeizou/textarea5/textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/safe-buffer'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
        - install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "buffer": false }
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/string_decoder/lib/string_decoder.js 26:13-42
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/lib/_stream_readable.js 163:40-80 325:38-78
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/node_modules/readable-stream/readable-browser.js 1:10-63
 @ ./textarea-markdown/node_modules/readable-web-to-node-stream/lib/index.js 4:26-52
 @ ./textarea-markdown/node_modules/file-type/browser.js 2:0-68 36:37-60
 @ ./textarea-markdown/src/textarea-markdown.js 3:0-44
 @ ./src/index.js 1:0-73 5:6-22

ERROR in node:buffer
Module build failed: UnhandledSchemeError: Reading from "node:buffer" is not handled by plugins (Unhandled scheme).
Webpack supports "data:" and "file:" URIs by default.
You may need an additional plugin to handle "node:" URIs.
    at /Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:832:25
    at Hook.eval [as callAsync] (eval at create (/Users/nakamurakeizou/textarea5/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/Users/nakamurakeizou/textarea5/node_modules/tapable/lib/Hook.js:18:14)
    at Object.processResource (/Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:829:8)
    at processResource (/Users/nakamurakeizou/textarea5/node_modules/loader-runner/lib/LoaderRunner.js:220:11)
    at iteratePitchingLoaders (/Users/nakamurakeizou/textarea5/node_modules/loader-runner/lib/LoaderRunner.js:171:10)
    at runLoaders (/Users/nakamurakeizou/textarea5/node_modules/loader-runner/lib/LoaderRunner.js:398:2)
    at NormalModule._doBuild (/Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:819:3)
    at NormalModule.build (/Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:963:15)
    at /Users/nakamurakeizou/textarea5/node_modules/webpack/lib/Compilation.js:1371:12
 @ ./textarea-markdown/node_modules/file-type/browser.js 1:0-35 44:27-38
 @ ./textarea-markdown/src/textarea-markdown.js 3:0-44
 @ ./src/index.js 1:0-73 5:6-22

ERROR in node:stream
Module build failed: UnhandledSchemeError: Reading from "node:stream" is not handled by plugins (Unhandled scheme).
Webpack supports "data:" and "file:" URIs by default.
You may need an additional plugin to handle "node:" URIs.
    at /Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:832:25
    at Hook.eval [as callAsync] (eval at create (/Users/nakamurakeizou/textarea5/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
    at Object.processResource (/Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:829:8)
    at processResource (/Users/nakamurakeizou/textarea5/node_modules/loader-runner/lib/LoaderRunner.js:220:11)
    at iteratePitchingLoaders (/Users/nakamurakeizou/textarea5/node_modules/loader-runner/lib/LoaderRunner.js:171:10)
    at runLoaders (/Users/nakamurakeizou/textarea5/node_modules/loader-runner/lib/LoaderRunner.js:398:2)
    at NormalModule._doBuild (/Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:819:3)
    at NormalModule.build (/Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:963:15)
    at /Users/nakamurakeizou/textarea5/node_modules/webpack/lib/Compilation.js:1371:12
    at NormalModule.needBuild (/Users/nakamurakeizou/textarea5/node_modules/webpack/lib/NormalModule.js:1253:32)
 @ ./textarea-markdown/node_modules/file-type/core.js 1503:33-54
 @ ./textarea-markdown/node_modules/file-type/browser.js 3:0-91 37:24-46 44:8-26 47:0-51:19 47:0-51:19 47:0-51:19 47:0-51:19
 @ ./textarea-markdown/src/textarea-markdown.js 3:0-44
 @ ./src/index.js 1:0-73 5:6-22

4 errors have detailed information that is not shown.
Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.

webpack 5.75.0 compiled with 6 errors in 493 ms

試したこと

webpack.config.jsに以下の設定を追加することでコンパイルは上手くいきました。

: 参考 sindresorhus/file-type#502

 target: "node"

ですがtarget: "node"としているためかブラウザで実行すると動作しませんでした。

# ブラウザのコンソール

Uncaught ReferenceError: require is not defined

ここから色々情報を集めていた時にmagic-bytes.jsを見つけ、textarea-markdownにインストールしてみたところ、コンパイルも問題なくできてファイルの種類の判別ができたのでこちらを使ったほうが無難だと思ったのですがどうでしょうか?色々調べていく中でfile-typeはNodejs用であり、ブラウザで動かすのは不向きなようにも感じました。

@komagata
Copy link
Member

@keiz1213 動くのであればどちらのライブラリでも大丈夫です〜

@keiz1213
Copy link
Contributor Author

@komagata

回答ありがとうございます!magic-bytes.jsを使って進めたいと思います。

重ねて質問なのですが、ファイルのサイズを見やすく表示するためfilesize.jsというパッケージを追加しようと考えているのですが、textarea-markdownにインストールする時にエラーが出てしまいます。
--legacy-peer-depsを付けてインストールするとエラーが起きず正常に動くのですがこの方法で大丈夫でしょうか?

npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR! 
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/webpack
npm ERR!   peer webpack@"^5.1.0" from [email protected]
npm ERR!   node_modules/terser-webpack-plugin
npm ERR!     terser-webpack-plugin@"^5.1.3" from [email protected]
npm ERR!   peer webpack@"^4.0.0 || ^5.0.0" from [email protected]
npm ERR!   node_modules/webpack-dev-middleware
npm ERR!     webpack-dev-middleware@"^5.3.1" from [email protected]
npm ERR!     node_modules/webpack-dev-server
npm ERR!       dev webpack-dev-server@"^4.11.1" from the root project
npm ERR!   2 more (webpack-dev-server, the root project)
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer webpack@"2 || 3" from [email protected]
npm ERR! node_modules/babel-loader
npm ERR!   dev babel-loader@"^7.1.2" from the root project
npm ERR! 
npm ERR! Conflicting peer dependency: [email protected]
npm ERR! node_modules/webpack
npm ERR!   peer webpack@"2 || 3" from [email protected]
npm ERR!   node_modules/babel-loader
npm ERR!     dev babel-loader@"^7.1.2" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR! 
npm ERR! See /Users/nakamurakeizou/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/nakamurakeizou/.npm/_logs/2022-11-13T04_02_22_092Z-debug-0.log

@komagata
Copy link
Member

@keiz1213

-legacy-peer-depsを付けてインストールするとエラーが起きず正常に動くのですがこの方法で大丈夫でしょうか?

大丈夫ではないです。環境に問題があるのか、もしくは追加したライブラリに問題があるのか、他のライブラリに変えるなど必要かもです。

@keiz1213
Copy link
Contributor Author

@komagata
回答ありがとうございます。

大丈夫ではないです。環境に問題があるのか、もしくは追加したライブラリに問題があるのか、他のライブラリに変えるなど必要かもです。

なるほどです。わかりました。

改めてエラーメッセージを読み、babel loaderを現状の7系から最新バージョン(9系)にアップデートするとエラーがでずにライブラリをインストールできるようになりましたが、この方法で進めてもよろしいでしょうか?

何度もお手数おかけして申し訳ありませんがよろしくお願い致します。

@komagata
Copy link
Member

@keiz1213

改めてエラーメッセージを読み、babel loaderを現状の7系から最新バージョン(9系)にアップデートするとエラーがでずにライブラリをインストールできるようになりましたが、この方法で進めてもよろしいでしょうか?

はい、お願いします〜

@keiz1213
Copy link
Contributor Author

@komagata

ありがとうございます!アップデートして進めていきます💪

@keiz1213 keiz1213 force-pushed the feature/rendering-non-image-file-into-a-link branch from 467ea9c to 572e0b5 Compare November 16, 2022 11:28
@keiz1213 keiz1213 marked this pull request as ready for review November 16, 2022 14:38
@keiz1213
Copy link
Contributor Author

@shirotamaki

お疲れさまです!こちらレビューをお願いできますでしょうか?全然急いでないのでいつでも大丈夫です😄

@shirotamaki
Copy link
Contributor

@keiz1213
お疲れさまです!ご依頼いただきありがとうございます!
20日(日)までには確認できると思います!よろしくお願いします〜😊

Copy link
Contributor

@shirotamaki shirotamaki left a comment

Choose a reason for hiding this comment

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

@keiz1213
お疲れ様です!動作確認できました🎉
npmへのPRも拝見しました👀 今回使用しているnpmを採用された経緯が当PR上のコメントやり取りで知ることができわかりやすかったです〜!Approveいたします!😊

@keiz1213
Copy link
Contributor Author

@shirotamaki

確認ありがとうございます!

今回使用しているnpmを採用された経緯が当PR上のコメントやり取りで知ることができわかりやすかったです〜!

何度も質問することになると思ったのでPRの方に残してて良かったです😄

@komagata
こちらチームの方からapproveいただきましたのでご確認お願いいたします!

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

確認させて頂きました。OKです〜🙆‍♂️

難しいIssueありがとうございました〜!

@komagata komagata merged commit 2909b19 into main Nov 21, 2022
@komagata komagata deleted the feature/rendering-non-image-file-into-a-link branch November 21, 2022 13:25
@github-actions github-actions bot mentioned this pull request Nov 21, 2022
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants