Migrate to Vite, Vitest, Add Linter #262

rasyidf · 2022-07-21T18:17:16Z

Pre-flight checklist

Update Dependencies and use vite to speed up development.

React 18
material-ui/core -> mui/material
react scripts to vite
jest to vitest.

Unit tests for all non-trivial changes
Tested locally

SkalskiP · 2022-07-22T06:26:38Z

Hi @rasyidf, that's a lot of changes :) I know, that most of them are related to adding linter, but I still need to go through that. I'll try to put any PR-related comments today.

rasyidf · 2022-07-22T08:25:42Z

Hello, I'm sorry that was a bulk changes. the reason I changed many things, It's mostly because, I've been struggling to develop the app. then trying to fix it by replacing some components with newer one, changing the build tools. and testing tools.

so it will be able to run without any error when using newer npm.

rasyidf · 2022-07-22T08:30:42Z

the huge changelog is mostly because of package-lock.json

SkalskiP · 2022-07-22T14:22:55Z

I understand, I actually did some of these changes on a separate branch this week, but it is not merged. I'm traveling today for 3 hours. I hope that will be enough to go through all the changes and review them.

.github/workflows/coverage.yml

index.html

src/index.tsx

src/views/PopupView/InsertLabelNamesPopup/InsertLabelNamesPopup.tsx

src/staticModels/EditorModel.ts

src/utils/RectUtil.ts

src/views/MainView/ImagesDropZone/ImagesDropZone.tsx

src/views/PopupView/ImportLabelPopup/ImportLabelPopup.tsx

SkalskiP · 2022-07-22T17:29:38Z

Hi, @rasyidf I see a lot of cool changes here. I definitely like:

Upgrading React to version 18
Adding linter! This is great!
Vitt is really fast!

Problems that I see that are the main blockers for me now:

Code coverage seems not to work properly
Upgrading MUI caused the removal of input styles in the LabelName popup
Upgrading Scrollbars caused scrolls to be visible when you zoom the image and drag it.

rasyidf · 2022-07-22T18:33:54Z

I can work with the coverge asap.

that's a regression. and the label i guess it's because of breaking changes. I can fix that

rasyidf · 2022-07-22T20:39:10Z

the TextField is fixed. solving another bug.

codecov-commenter · 2022-07-23T08:13:06Z

Codecov Report

Merging #262 (5c0a26f) into develop (d6e03cb) will increase coverage by 1.51%.
The diff coverage is 6.97%.

@@             Coverage Diff             @@
##           develop     #262      +/-   ##
===========================================
+ Coverage    12.63%   14.14%   +1.51%     
===========================================
  Files          135      149      +14     
  Lines         4030     4361     +331     
  Branches       719      792      +73     
===========================================
+ Hits           509      617     +108     
- Misses        3521     3741     +220     
- Partials         0        3       +3

Impacted Files	Coverage Δ
src/ai/ObjectDetector.ts	`26.66% <0.00%> (+26.66%)`	⬆️
src/ai/PoseDetector.ts	`26.66% <0.00%> (+26.66%)`	⬆️
src/logic/import/yolo/YOLOImporter.ts	`39.21% <0.00%> (+13.02%)`	⬆️
src/logic/import/yolo/YOLOUtils.ts	`80.95% <ø> (+2.00%)`	⬆️
src/staticModels/EditorModel.ts	`100.00% <ø> (ø)`
src/utils/CanvasUtil.ts	`5.88% <0.00%> (+5.88%)`	⬆️
src/utils/RectUtil.ts	`11.11% <0.00%> (+2.97%)`	⬆️
src/utils/VirtualListUtil.ts	`0.00% <0.00%> (ø)`
src/views/Common/VirtualList/VirtualList.tsx	`0.00% <ø> (ø)`
src/views/EditorView/Editor/Editor.tsx	`0.00% <ø> (-1.93%)`	⬇️
... and 162 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6e03cb...5c0a26f. Read the comment docs.

SkalskiP · 2022-07-23T08:41:23Z

@rasyidf I'll take a look at the changes today. I need to take care of a few things now, but I'll be back in more or less 2 hours.

rasyidf · 2022-07-23T08:44:06Z

@rasyidf I'll take a look at the changes today. I need to take care of a few things now, but I'll be back in more or less 2 hours.

take your time.

SkalskiP · 2022-07-23T11:18:23Z

Hi, @rasyidf I took a look at your progress and I'm super excited about that PR! 🚀 I decided to create a checklist, to make our life easier.

Upgrading React to v18
Adding linter
Fix test code coverage
Upgrading MUI
- Fix text input in InsertLabelNamesPopup
- Fix StyledTooltip background color is changed. You can see that if you hover over the GitHub or Medium icon on the main site. It used to be darker now it light gray.
Fix scrollbars. When you zoom the image and start to drag it you see scrollbars, we want to prevent that.
Problems with loading COCO JSON from disk. JSON files are grayed out.

I noticed that the loading SSD model does not work. It may not be related to your PR. I'll need to take a look at that later.

src/staticModels/EditorModel.ts

src/views/MainView/MainView.scss

src/views/MainView/MainView.tsx

src/views/PopupView/ImportLabelPopup/ImportLabelPopup.tsx

rasyidf · 2022-07-23T11:47:13Z

Somehow I can't reproduce the scrollbar bug. the app is working fine in my localhost

SkalskiP · 2022-07-23T12:02:21Z

@rasyidf Please set public static viewPortActionsDisabled as false. And I guess we could try to merge. I'll try to figure out those scrollbars on my own later ;)

rasyidf · 2022-07-23T12:06:18Z

I've done changing. I guess that's everything @SkalskiP .

SkalskiP · 2022-07-23T12:25:05Z

@rasyidf great! So in that case I'm merging to develop. Changes should be on develop in a few minutes if everything works out ok. 🤞

SkalskiP · 2022-07-23T12:37:50Z

@rasyidf develop build failed. I'll try to understand why.

SkalskiP · 2022-07-23T13:51:35Z

@rasyidf I managed to build it, and it seems that it even deployed. But when I go to https://develop.makesense.ai/ the website is empty there is nothing in root div.

SkalskiP · 2022-07-23T14:09:37Z

@rasyidf I managed to replicate the same empty root div locally. I did according to this tutorial: https://vitejs.dev/guide/static-deploy.html. Added preview script and than run:

$ npm run build
$ npm run preview

Same result, empty page. Any ideas how to fix that?

rasyidf · 2022-07-23T14:14:36Z

@rasyidf I managed to replicate the same empty root div locally. I did according to this tutorial: https://vitejs.dev/guide/static-deploy.html. Added preview script and than run:
$ npm run build
$ npm run preview
Same result, empty page. Any ideas how to fix that?

may i know what hosting type are you using?

SkalskiP · 2022-07-23T14:23:46Z

@rasyidf I managed to replicate the same empty root div locally. I did according to this tutorial: https://vitejs.dev/guide/static-deploy.html. Added preview script and than run:
$ npm run build
$ npm run preview
Same result, empty page. Any ideas how to fix that?
may i know what hosting type are you using?

Sure, Amplify

SkalskiP · 2022-07-23T14:25:41Z

I think it is because of chanking. When I overwrite the current vite.config.js.

import { defineConfig } from 'vite'
import react from "@vitejs/plugin-react";

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [react()]
})

It loads properly.

rasyidf · 2022-07-23T14:28:18Z

aha, that's something I missed.

there's build error I skipped.

i guess this is also another reason.

the app can't find aws nock.

SkalskiP · 2022-07-23T14:31:02Z

Yup, I was also thinking about it. You think it is not chunking issue?

rasyidf · 2022-07-23T14:32:11Z

I've fixed it by adding mock-aws-s3, aws-sdk and nock ad devDependency

SkalskiP · 2022-07-23T14:49:27Z

I committed changes to develop, adding those dependencies. Let's see if it'll work. It worked locally.

SkalskiP · 2022-07-23T16:45:21Z

@rasyidf take a look at: https://develop.makesense.ai I think everything works now! Thank you very much for your contribution 🚀🚀🚀 I'm very happy and excited!

telkomdev-rasyid added 6 commits July 22, 2022 00:51

Migrate to Vite, Vitest, Add Linter

7401a76

Update Testing

6b592b4

Fix Coverage

f0f2640

Update Actions Version

0f09396

Fix Coverage Test Failed

f13cb04

fix npm lockfile error

621679b

SkalskiP requested changes Jul 22, 2022

View reviewed changes

telkomdev-rasyid added 5 commits July 23, 2022 04:04

Fix StyledTextField

1462daf

Reconfigure Testing and Coverage

f9e82bd

Fix Test and Coverage

a27acd2

Change TextField to Standard

08dfb65

Update Linter to allow object initializer

ad1c739

fix some UI bug

d6ff459

SkalskiP requested changes Jul 23, 2022

View reviewed changes

src/staticModels/EditorModel.ts Outdated Show resolved Hide resolved

src/views/MainView/MainView.scss Show resolved Hide resolved

src/views/MainView/MainView.tsx Outdated Show resolved Hide resolved

src/views/PopupView/ImportLabelPopup/ImportLabelPopup.tsx Show resolved Hide resolved

telkomdev-rasyid added 2 commits July 23, 2022 18:25

Fix Json Grayed Out

7639a08

Fix Styled Component

1911711

Revert initializer

5c0a26f

SkalskiP merged commit 49bc1cf into SkalskiP:develop Jul 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to Vite, Vitest, Add Linter #262

Migrate to Vite, Vitest, Add Linter #262

rasyidf commented Jul 21, 2022 •

edited

SkalskiP commented Jul 22, 2022

rasyidf commented Jul 22, 2022

rasyidf commented Jul 22, 2022

SkalskiP commented Jul 22, 2022

SkalskiP commented Jul 22, 2022

rasyidf commented Jul 22, 2022

rasyidf commented Jul 22, 2022

codecov-commenter commented Jul 23, 2022 •

edited

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022

SkalskiP commented Jul 23, 2022 •

edited

rasyidf commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022 •

edited

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022 •

edited

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022 •

edited

SkalskiP commented Jul 23, 2022 •

edited

SkalskiP commented Jul 23, 2022

Migrate to Vite, Vitest, Add Linter #262

Migrate to Vite, Vitest, Add Linter #262

Conversation

rasyidf commented Jul 21, 2022 • edited

Pre-flight checklist

SkalskiP commented Jul 22, 2022

rasyidf commented Jul 22, 2022

rasyidf commented Jul 22, 2022

SkalskiP commented Jul 22, 2022

SkalskiP commented Jul 22, 2022

rasyidf commented Jul 22, 2022

rasyidf commented Jul 22, 2022

codecov-commenter commented Jul 23, 2022 • edited

Codecov Report

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022

SkalskiP commented Jul 23, 2022 • edited

rasyidf commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022 • edited

SkalskiP commented Jul 23, 2022

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022 • edited

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 23, 2022 • edited

SkalskiP commented Jul 23, 2022 • edited

SkalskiP commented Jul 23, 2022

rasyidf commented Jul 21, 2022 •

edited

codecov-commenter commented Jul 23, 2022 •

edited

SkalskiP commented Jul 23, 2022 •

edited

rasyidf commented Jul 23, 2022 •

edited

rasyidf commented Jul 23, 2022 •

edited

rasyidf commented Jul 23, 2022 •

edited

SkalskiP commented Jul 23, 2022 •

edited