-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
Migrate to Vite, Vitest, Add Linter #262
Conversation
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. |
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. |
the huge changelog is mostly because of package-lock.json |
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. |
src/views/PopupView/InsertLabelNamesPopup/InsertLabelNamesPopup.tsx
Outdated
Show resolved
Hide resolved
Hi, @rasyidf I see a lot of cool changes here. I definitely like:
Problems that I see that are the main blockers for me now:
|
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 |
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
@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. |
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.
|
Somehow I can't reproduce the scrollbar bug. the app is working fine in my localhost |
@rasyidf Please set |
I've done changing. I guess that's everything @SkalskiP . |
@rasyidf develop build failed. I'll try to understand why. |
@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. |
@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 |
I think it is because of chanking. When I overwrite the current import { defineConfig } from 'vite'
import react from "@vitejs/plugin-react";
// https://vitejs.dev/config/
export default defineConfig({
plugins: [react()]
}) It loads properly. |
Yup, I was also thinking about it. You think it is not chunking issue? |
I committed changes to develop, adding those dependencies. Let's see if it'll work. It worked locally. |
@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! |
Pre-flight checklist
Update Dependencies and use vite to speed up development.