Replace NextJS with `Vite` #1431

Da-Colon · 2024-03-11T17:02:22Z

remove next config and types
Add vite and react-router-dom
Add vite config
Create index.html (update with metadata from next/head)
Create main.tsx (entry point)
Add CSS imports, providers to entry point
Create router.tsx to handle page router
Add DAOController and Layout element to Router
Add Pages to Router (A few index -> default component name file changes)
Replace next's Script with custom hook
Replace next Link Component with React Router Link
Replace process.env -> import.meta with update ENV names
Replace next/image component with Chakra Image component
Replace next/router push instances with useNavigate
Replace package.json next scripts with vite runner

Testing

Note: Deployed preview will not work.

I deployed this branch manually on netlify. Here is the link: https://test-vite-fractalframework.netlify.app/

…etup)

…o lesgoo/switchtovite

- remove next config and types - Add vite and react-router-dom - Add vite config - Create index.html (update with metadata from next/head) - Create main.tsx (entry point) - Add CSS imports, providers to entry point - Create router.tsx to handle page router - Add DAOController and Layout element to Router - Add Pages to Router (A few index -> default component name file changes) - Replace next's Script with custom hook - Replace next Link Component with React Router Link - Replace process.env -> import.meta with update ENV names - Replace next/image component with Chakra Image component - Replace next/router push instances with `useNavigate` - Replace package.json next scripts with vite runner

netlify · 2024-03-11T17:02:39Z

✅ Deploy Preview for test-vite-fractalframework ready!

Name	Link
🔨 Latest commit	`c152334`
🔍 Latest deploy log	https://app.netlify.com/sites/test-vite-fractalframework/deploys/65f1aff52e075b0008db3208
😎 Deploy Preview	https://deploy-preview-1431--test-vite-fractalframework.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Da-Colon

Test everything workflow you can think of. Should only notice the app being smoother everywhere. report any weirdness and I'll look into it.

Da-Colon · 2024-03-11T17:04:17Z

src/router.tsx

This was surprisingly pretty straight forward to create. Just takes the 'index' of each page folder formally the page.tsx and place as the element.

Da-Colon · 2024-03-11T17:05:44Z

.env

Don't forget to update local env files as well from NEXT_PUBLIC_ -> VITE_APP_

Da-Colon · 2024-03-11T17:06:55Z

src/hooks/utils/useETHLizardsScript.tsx

I cannot test this since Goerli is no longer supported. Should we just remove?

Yeah just remove this script - it was purely for user testing on Goerli, we don't need it anymore

Da-Colon · 2024-03-11T17:08:25Z

vite.config.ts

So far in my testing, all I've had to update here is the plugin for nodepolyfils. This was fixing a 'process is not defined' and 'global is not defined' errors when setting up

mudrila

Amazing job!
Wasn't expecting it would be relatively simple. Have bunch of comments for changes, but other than that - looks good! Gonna test it in a moment

src/components/pages/DaoDashboard/Info/ParentLink.tsx

src/components/pages/DaoSettings/components/Metadata/index.tsx

src/components/ui/cards/DAOInfoCard.tsx

mudrila · 2024-03-11T17:27:07Z

src/helpers/errorLogging.ts

@@ -6,13 +6,13 @@ import * as Sentry from '@sentry/react';
 export function initErrorLogging() {
  if (
    process.env.NODE_ENV === 'development' ||
-    process.env.NEXT_PUBLIC_SENTRY_DSN_URL === undefined
+    import.meta.env.VITE_APP_SENTRY_DSN_URL === undefined


I know this is not related to the topic of this PR, but we need to make sure it is not an empty string as well. We have VITE_APP_SENTRY_DSN_URL="" in .env and I think it is good idea to keep that on env file, but then we need to make sure it is not an empty string

mudrila · 2024-03-11T17:30:56Z

src/hooks/utils/useETHLizardsScript.tsx

Yeah just remove this script - it was purely for user testing on Goerli, we don't need it anymore

src/pages/DAOController.tsx

mudrila · 2024-03-11T17:33:20Z

src/pages/daos/[daoAddress]/proposal-templates/new/index.tsx

-  const { push } = useRouter();
-  const searchParams = useSearchParams();
+  const navigate = useNavigate();
+  let [searchParams] = useSearchParams();


This should be const I believe

docs shows let? https://reactrouter.com/en/main/hooks/use-search-params.

Yeah but, please make it a const cause we're not changing it.

- replace NextLink -> RouterLink - remove ETHLizardsScript

- https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated

Da-Colon · 2024-03-11T18:05:05Z

@mudrila All your comments have been address, sir. (+some)

adamgall

Can we use the new VITE_APP_NAME env var in all the places in the codebase that we were originally using the APP_NAME constant defined in src/constants/common.ts? And then, remove that constant from common.ts?

src/vite-env.d.ts

index.html

src/pages/DAOController.tsx

src/providers/NetworkConfig/rainbow-kit.config.ts

adamgall

The deployed site doesn't load if you refresh the page on any non-root URL.

This is known and expected.

There's a simple fix for Netlify hosting, which allows us to keep the BrowserRouter and not need to switch to a HashRouter.

See this commit from the fractal-interface-2 repo: decentdao/fractal-interface-2@6bc7702

adamgall · 2024-03-11T21:09:07Z

index.html

+    />
+    <meta
+      name="description"
+      content="Are you outgrowing your Multisig? Fractal extends Safe treasuries into on-chain hierarchies of permissions, token flows, and governance."


Blah, I think that either this should be in an env var, or we just hardcode "Fractal" back into the places where we are using VITE_APP_NAME.

Why have some of this metadata hardcoded and others controlled by environment?

But whatever, it's fine for now.

adamgall · 2024-03-11T21:11:15Z

vite.config.mts

+export default defineConfig({
+  plugins: [nodePolyfills(), react()],
+  server: {
+    port: 3000,


@Da-Colon just wondering, does it actually matter to force this to run on 3000? THIS AIN'T A NEXT APP ANY LONGER BOYYEEEEEE

lol fair. more for me and my history autocomplete :D

I don't really have a preference. btw we can definately just go with the defaults of vite.

My preference is that we just go with the vite default

adamgall · 2024-03-11T21:21:47Z

src/metadata/lizzardsDAO/index.ts

Does this whole file and associated other lizard-specific files get removed too? I'm not sure where we stand on that....

@mudrila @tomstuart123 ^^^^

The export from this file isn't being consumed anywhere, so from a codebase perspective, it's safe to delete this file. I wonder if we should hold on to it for future use though...

adamgall · 2024-03-11T21:24:16Z

src/pages/daos/[daoAddress]/proposal-templates/new/index.tsx

-  const { push } = useRouter();
-  const searchParams = useSearchParams();
+  const navigate = useNavigate();
+  let [searchParams] = useSearchParams();


Yeah but, please make it a const cause we're not changing it.

adamgall · 2024-03-12T21:57:50Z

@Da-Colon can you explain the difference between chakra's <Link> component and react-router-dom's <Link> component? There seems to be extensive use of both in the codebase, and I'm not sure what the distinction is between them.

This commit has me scratching my head... why change the href to to on these chakra <Link>'s and not others?

Da-Colon · 2024-03-12T22:12:45Z

@Da-Colon can you explain the difference between chakra's <Link> component and react-router-dom's <Link> component? There seems to be extensive use of both in the codebase, and I'm not sure what the distinction is between them.

This commit has me scratching my head... why change the href to to on these chakra <Link>'s and not others?

@adamgall

Chakra-UI's Link basically is a stylable <a> element
React-Router's Link adds React Router's functionality to the <a> element. https://reactrouter.com/en/main/components/link

Switching to to property for these allowed it to pick up that we are just navigating to a different page rather than reloading the document

As far as the various uses. Looking through how I've updated it.

React-Router's Link is used when the navigation is internal

Chakra-UI's Link is used when navigation is external or needs to be styled.

In the case of needing to be styled and internal. Chakra's as property is used. This allows Chakra's styling ability to be used along side React-Router's properties (why you saw these switch to to property as well)

adamgall · 2024-03-13T12:52:58Z

@Da-Colon this makes a lot of sense, thank you

tomstuart123 · 2024-03-19T17:51:54Z

So I think this is good to go

Successful Test cases for both multisig + token dao

Create a DAO
delegate (token only)
vote / sign
wait for timelock
execute

these were for subdao, token send and create a proposal flow

just a note - I did have old issues reccur with subdao activity but david pointed me to a seperate PR for that - https://discord.com/channels/839548879216181309/1174434485043998860/1219698481091055699

final note - I did get this error that I had never seen before. Hard to replicate

this was the only new thing and happened pretty rarely. I think we're good to go on this

Da-Colon · 2024-03-19T17:55:29Z

New Error you are experiencing is this?

https://github.com/orgs/decentdao/projects/32/views/3?pane=issue&itemId=56621736

Da-Colon added 10 commits March 7, 2024 15:32

Remove App Directory

59f570d

updates to next config

215d2be

refactor AppRouter pages for use in Page Router

46104dc

refactor DAO Layout into Compontent that wraps apps (needed for new s…

3a6c1d8

…etup)

addresses issues with items loading now that component isn't remounted

06e0067

move imports to _app.tsx (top of tree)

6f17b18

Merge branch 'develop' of github.com:decent-dao/fractal-interface int…

841c10c

…o lesgoo/switchtovite

remove console log

29c045c

remove export config property (for now)

8b400d0

Da-Colon self-assigned this Mar 11, 2024

Da-Colon linked an issue Mar 11, 2024 that may be closed by this pull request

Static Upgrade #1422

Closed

add env typing

fa55121

Da-Colon commented Mar 11, 2024

View reviewed changes

Da-Colon requested review from adamgall, mudrila and tomstuart123 March 11, 2024 17:17

Da-Colon mentioned this pull request Mar 11, 2024

Static Upgrade #1422

Closed

mudrila requested changes Mar 11, 2024

View reviewed changes

Da-Colon added 5 commits March 11, 2024 13:49

Kyrylo comments updates

4119509

- replace NextLink -> RouterLink - remove ETHLizardsScript

run lint fix

2ce2934

remove Next SSR ClientOnly Component bandaid and 'use client' flags

780799b

update lock file

b04ca55

configure vite for depreciated warning. Vite CJS Node API deprecated

d92d292

- https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated

adamgall requested changes Mar 11, 2024

View reviewed changes

Da-Colon added 2 commits March 11, 2024 14:51

Fix for react-router-dom BrowserRouter on Netlify

03f8151

replace APP_NAME const, with VITE_APP_NAME env

0e4602c

Da-Colon and others added 3 commits March 11, 2024 16:57

add lint:fix script

04f302a

Remove next ignores

a576237

Standardize lint and pretty scritps

c0ddfda

adamgall reviewed Mar 11, 2024

View reviewed changes

adamgall requested changes Mar 11, 2024

View reviewed changes

let -> const

18452a3

adamgall approved these changes Mar 11, 2024

View reviewed changes

Reorder package.json deps to be alphabetical

f7a70f6

mudrila approved these changes Mar 12, 2024

View reviewed changes

Da-Colon and others added 2 commits March 12, 2024 16:03

add div element missing from ClientOnly removal

9edf39c

Fix env var references in the <head>

45150fa

adamgall self-requested a review March 12, 2024 20:22

Da-Colon added 4 commits March 12, 2024 16:37

fix heirarchy loading issue

40a656c

fix page header breadcrumb link re-rendering dao page when clicked

d9f1f49

fix new proposal-templates route

30916d4

switch last few Router Links to to property from href

8935596

adamgall approved these changes Mar 12, 2024

View reviewed changes

Da-Colon and others added 2 commits March 13, 2024 09:15

Switch Page navigation to React-Router Link component

a5803ee

Clean up some directory declarations in gitignore

c152334

Da-Colon changed the base branch from lesgoo/switchtovite to develop March 13, 2024 14:18

adamgall merged commit 0cddf55 into develop Mar 13, 2024
7 checks passed

adamgall deleted the lesgoo/switchtovite-removeNextJS branch March 13, 2024 16:40

adamgall mentioned this pull request Mar 19, 2024

Release/20240319 #1471

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace NextJS with `Vite` #1431

Replace NextJS with `Vite` #1431

Da-Colon commented Mar 11, 2024 •

edited

Loading

netlify bot commented Mar 11, 2024 •

edited

Loading

Da-Colon left a comment

Da-Colon Mar 11, 2024

Da-Colon Mar 11, 2024

Da-Colon Mar 11, 2024

mudrila Mar 11, 2024

Da-Colon Mar 11, 2024

mudrila left a comment

mudrila Mar 11, 2024

mudrila Mar 11, 2024

mudrila Mar 11, 2024

Da-Colon Mar 11, 2024

adamgall Mar 11, 2024

Da-Colon commented Mar 11, 2024

adamgall left a comment

adamgall left a comment

adamgall Mar 11, 2024

adamgall Mar 11, 2024

Da-Colon Mar 11, 2024

Da-Colon Mar 11, 2024

adamgall Mar 11, 2024

adamgall Mar 11, 2024

Da-Colon Mar 11, 2024

adamgall Mar 11, 2024

adamgall Mar 11, 2024

adamgall commented Mar 12, 2024

Da-Colon commented Mar 12, 2024

adamgall commented Mar 13, 2024

tomstuart123 commented Mar 19, 2024

Da-Colon commented Mar 19, 2024

Replace NextJS with Vite #1431

Replace NextJS with Vite #1431

Conversation

Da-Colon commented Mar 11, 2024 • edited Loading

Testing

netlify bot commented Mar 11, 2024 • edited Loading

✅ Deploy Preview for test-vite-fractalframework ready!

Da-Colon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mudrila left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Da-Colon commented Mar 11, 2024

adamgall left a comment

Choose a reason for hiding this comment

adamgall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamgall commented Mar 12, 2024

Da-Colon commented Mar 12, 2024

adamgall commented Mar 13, 2024

tomstuart123 commented Mar 19, 2024

Da-Colon commented Mar 19, 2024

Replace NextJS with `Vite` #1431

Replace NextJS with `Vite` #1431

Da-Colon commented Mar 11, 2024 •

edited

Loading

netlify bot commented Mar 11, 2024 •

edited

Loading