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

Bump frontend deps #7813

Merged
merged 13 commits into from
May 27, 2024
Merged

Bump frontend deps #7813

merged 13 commits into from
May 27, 2024

Conversation

MichaelBuessemeyer
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer commented May 21, 2024

This PR updates some outdated frontend dependencies and replaces the markdown library with react-markdown. Moreover, prop types, esbuild loader, protobuffjs, and onnx web runtime are updated.

In order to fix the frontend tests, the new markdown library needs to be loaded lazily. Thus, I created the markdown_adapter.tsx file.

URL of deployed dev instance (used for testing):

Steps to test:

  • Check whether wk works on a rudimentary level.
  • Check whether markdown is rendered correctly in: (Already tested by me but re-testing some of them might be helpful)
    • the publication gallery
    • the annotation description in the info tab an annotation
    • task list view of a user
    • new task info modal (shown upon opening a task)
    • In the annotation view: Hovering the burger menu on the left to an annotation title should show the description

TODOs:

  • Test whether the current markdown rendering matches the previous settings options={{ html: false, breaks: true, linkify: true, }

Issues:

  • No issues exist for this

(Please delete unneeded items, merge only when none are left open)

  • [ ] Updated changelog I'd say it is not needed as this is not user-facing.

@MichaelBuessemeyer MichaelBuessemeyer marked this pull request as ready for review May 23, 2024 14:17
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

awesome! the changes look very good. only left some smaller comments. will test once these are addressed.

package.json Outdated Show resolved Hide resolved
@@ -83,18 +77,21 @@ module.exports = function (env = {}) {
],
},
{
test: /\.tsx?$/,
Copy link
Member

Choose a reason for hiding this comment

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

why did you split tsx? into ts and tsx? the config looks the almost same or is there a difference because of the loader attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also a legacy from the debugging problems. Should be resolved now :)

@@ -166,6 +169,11 @@ module.exports = function (env = {}) {
},
optimization: {
minimize: env.production,
minimizer: [
new EsbuildPlugin({
target: buildTarget, // Syntax to transpile to (see options below for possible values)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
target: buildTarget, // Syntax to transpile to (see options below for possible values)
target: buildTarget,

Copy link
Member

Choose a reason for hiding this comment

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

did you sanity check the size of the js modules in the devtools (master vs this branch)?

Copy link
Contributor Author

@MichaelBuessemeyer MichaelBuessemeyer May 24, 2024

Choose a reason for hiding this comment

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

This branch:

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  8c3b61b77fee2ba4b6db.ttf (274 KiB)
  4a3eef3e0a61b7eb3eda.ttf (269 KiB)
  2f517e09eb2ca6650ff5.svg (730 KiB)
  7a8b4f130182d19a2d7c.svg (897 KiB)
  main.js (1.49 MiB)
  vendors~onnx.js (529 KiB)
  vendors~main.js (4.7 MiB)
  ort-training-wasm-simd.wasm (10.7 MiB)
  ort-wasm-simd-threaded.jsep.wasm (18.6 MiB)
  ort-wasm-simd-threaded.wasm (10.2 MiB)
  ort-wasm-simd.jsep.wasm (16.6 MiB)
  ort-wasm-simd.wasm (10.1 MiB)
  ort-wasm-threaded.wasm (9.36 MiB)
  ort-wasm.wasm (9.28 MiB)
  models/vit_l_0b3195_decoder_quantized.onnx (8.34 MiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (6.36 MiB)
      vendors~main.css
      vendors~main.js
      main.css
      main.js

On the master:

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  4a3eef3e0a61b7eb3eda.ttf (269 KiB)
  8c3b61b77fee2ba4b6db.ttf (274 KiB)
  2f517e09eb2ca6650ff5.svg (730 KiB)
  7a8b4f130182d19a2d7c.svg (897 KiB)
  main.js (1.53 MiB)
  vendors~onnx.js (547 KiB)
  vendors~main.js (4.76 MiB)
  models/vit_l_0b3195_decoder_quantized.onnx (8.34 MiB)
  ort-wasm-simd-threaded.wasm (9.5 MiB)
  ort-wasm-simd.wasm (9.55 MiB)
  ort-wasm-threaded.wasm (8.73 MiB)
  ort-wasm.wasm (8.8 MiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (6.45 MiB)
      vendors~main.css
      vendors~main.js
      main.css
      main.js

=> main size reduced :D
so it should be fine and it is faster than terser 🚀

loader: () => Promise<{ default: React.ComponentType<Props> }>,
) {
const InternalComponent = React.lazy(loader) as any;
return function AsyncComponent(_props: Props) {
Copy link
Member

Choose a reason for hiding this comment

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

why the _ prefix? since the var is used, it can simply be props, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, sorry. I had a lot of TS problems and this is a legacy of the debugging process 🙈

Copy link
Contributor Author

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Thanks for your feedback and also checking for potential upgrades of other packages. I applied all of it :)

loader: () => Promise<{ default: React.ComponentType<Props> }>,
) {
const InternalComponent = React.lazy(loader) as any;
return function AsyncComponent(_props: Props) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, sorry. I had a lot of TS problems and this is a legacy of the debugging process 🙈

package.json Outdated Show resolved Hide resolved
@@ -83,18 +77,21 @@ module.exports = function (env = {}) {
],
},
{
test: /\.tsx?$/,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also a legacy from the debugging problems. Should be resolved now :)

@@ -166,6 +169,11 @@ module.exports = function (env = {}) {
},
optimization: {
minimize: env.production,
minimizer: [
new EsbuildPlugin({
target: buildTarget, // Syntax to transpile to (see options below for possible values)
Copy link
Contributor Author

@MichaelBuessemeyer MichaelBuessemeyer May 24, 2024

Choose a reason for hiding this comment

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

This branch:

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  8c3b61b77fee2ba4b6db.ttf (274 KiB)
  4a3eef3e0a61b7eb3eda.ttf (269 KiB)
  2f517e09eb2ca6650ff5.svg (730 KiB)
  7a8b4f130182d19a2d7c.svg (897 KiB)
  main.js (1.49 MiB)
  vendors~onnx.js (529 KiB)
  vendors~main.js (4.7 MiB)
  ort-training-wasm-simd.wasm (10.7 MiB)
  ort-wasm-simd-threaded.jsep.wasm (18.6 MiB)
  ort-wasm-simd-threaded.wasm (10.2 MiB)
  ort-wasm-simd.jsep.wasm (16.6 MiB)
  ort-wasm-simd.wasm (10.1 MiB)
  ort-wasm-threaded.wasm (9.36 MiB)
  ort-wasm.wasm (9.28 MiB)
  models/vit_l_0b3195_decoder_quantized.onnx (8.34 MiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (6.36 MiB)
      vendors~main.css
      vendors~main.js
      main.css
      main.js

On the master:

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  4a3eef3e0a61b7eb3eda.ttf (269 KiB)
  8c3b61b77fee2ba4b6db.ttf (274 KiB)
  2f517e09eb2ca6650ff5.svg (730 KiB)
  7a8b4f130182d19a2d7c.svg (897 KiB)
  main.js (1.53 MiB)
  vendors~onnx.js (547 KiB)
  vendors~main.js (4.76 MiB)
  models/vit_l_0b3195_decoder_quantized.onnx (8.34 MiB)
  ort-wasm-simd-threaded.wasm (9.5 MiB)
  ort-wasm-simd.wasm (9.55 MiB)
  ort-wasm-threaded.wasm (8.73 MiB)
  ort-wasm.wasm (8.8 MiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (6.45 MiB)
      vendors~main.css
      vendors~main.js
      main.css
      main.js

=> main size reduced :D
so it should be fine and it is faster than terser 🚀

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

awesome 🥇

@MichaelBuessemeyer MichaelBuessemeyer merged commit e5e5854 into master May 27, 2024
2 checks passed
@MichaelBuessemeyer MichaelBuessemeyer deleted the bump-frontend-deps branch May 27, 2024 08:26
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.

2 participants