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

feat: vite upgrade #13195

Closed
wants to merge 39 commits into from
Closed

feat: vite upgrade #13195

wants to merge 39 commits into from

Conversation

yaldram
Copy link
Contributor

@yaldram yaldram commented Apr 21, 2022

Description

Fixes #12066

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

  • Tested Manually on the canvas

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@vercel
Copy link

vercel bot commented Apr 21, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview May 23, 2022 at 5:09AM (UTC)

@yaldram
Copy link
Contributor Author

yaldram commented Apr 21, 2022

  • Please rimraf the node_modules - rm -rf node_modules and then run yarn install and then run yarn start.

Two major issues that we are facing : -

  1. Derived properties are not working for the Select widget and TreeSelect widget. I think there is a problem under the src/workers/validations.ts file.
  2. Vite reload does not work for some .ts files here is one issue that I found maybe someone can look into it - App keeps refreshing with log: Server connection lost polling for restart vitejs/vite#4259

@yaldram
Copy link
Contributor Author

yaldram commented Apr 21, 2022

We removed the raw-loader webpack plugin now we are using vite-raw-plugin for loading the derived.js files. I have renamed all the derived.js files in the widgets to _derived.js and because we are using a new loader the regex under the parseDerivedProperties.ts was failing so as a work-around I added a dummy noop function in these files.

@yaldram
Copy link
Contributor Author

yaldram commented Apr 21, 2022

LOOM DEMO

Video Recording of the failing derivedProperties, it fails for the select, treeselect, multiselect, multitreeselect widgets.

@riodeuno riodeuno changed the base branch from typescript-upgrade to release April 22, 2022 10:38
@riodeuno riodeuno marked this pull request as draft April 22, 2022 10:38
@github-actions github-actions bot added the Enhancement New feature or request label Apr 22, 2022
@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

@yaldram
Copy link
Contributor Author

yaldram commented May 10, 2022

Some Caveats : -

  • We need to solve for the hot reload, re-load after the file changes - App keeps refreshing with log: Server connection lost polling for restart vitejs/vite#4259, it is also an issue for cypress test cases, when cases re-load.
  • @riodeuno encountered some issues with the routing when serving with build.
  • Jest test cases are failing for GlobalHotKeys related files.
  • We need to check if the vite-sentry plugin is working as expected, to pass the auth token to Sentry plugin inside the vite.config.ts, we need to have .env files. I am thinking of not using the Sentry plugin and use their components instead needs some investigation.
  • We need to check whether the vite-pwa plugin a replacement for workbox plugin is working as expected.
  • For the cypress test cases I have updated the tsconfig.json, we need to rebuild and run our application once so that we are sure everything is working as expected.
  • We need to make sure everything is working as expected.

@rishabhrathod01
Copy link
Contributor

Temporarily removing as a reviewer. Add again when PR is ready for review.

@github-actions github-actions bot added UI Builders Pod Issues that UI Builders face using appsmith UI Building Pod labels May 16, 2022
@taowen
Copy link

taowen commented May 22, 2022

@yaldram has the issue been fixed? taowen/vite-howto#4

@yaldram
Copy link
Contributor Author

yaldram commented May 22, 2022

@taowen Hey, no unfortunately we decided to move forward with cra-5 for now. Inline with this comment - vitejs/vite#7608 (comment) we might have to re-structure our codebase for faster hot reloads. But unfortunately we don't have any docs, guides from the vite team on how to restructure large projects with redux.

@riodeuno
Copy link
Contributor

Closing this in favour of #14000

@riodeuno riodeuno closed this May 30, 2022
@mohanarpit mohanarpit deleted the feat/vite-upgrade branch October 10, 2023 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Task A simple Todo UI Builders Pod Issues that UI Builders face using appsmith
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Update react-scripts to 5.0.0
4 participants