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

chore: vite migration #1955

Merged
merged 21 commits into from
Jul 20, 2023
Merged

chore: vite migration #1955

merged 21 commits into from
Jul 20, 2023

Conversation

flagrede
Copy link
Collaborator

@flagrede flagrede commented Jul 5, 2023

Description

Closes: regen-network/rnd-dev-team#1316

Pending Storybook dev server fix.

  • replace CRA by Vite
  • migrate storybook to V7

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • provided a link to the relevant issue or specification
  • provided instructions on how to test
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

How to test

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items
.

I have...

  • confirmed all author checklist items have been addressed
  • reviewed code correctness and readability
  • verified React components follow DRY principles
  • reviewed documentation is accurate
  • reviewed tests
  • manually tested (if applicable)

@netlify
Copy link

netlify bot commented Jul 5, 2023

Deploy Preview for regen-website ready!

Name Link
🔨 Latest commit 3a4e765
🔍 Latest deploy log https://app.netlify.com/sites/regen-website/deploys/64b7ab4ec6ce0b00086af940
😎 Deploy Preview https://deploy-preview-1955--regen-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@flagrede flagrede force-pushed the chore-1316-vite-migration branch 2 times, most recently from ac121b2 to ba35dab Compare July 5, 2023 15:01
@blushi blushi force-pushed the chore-1316-vite-migration branch from 0845e7b to b537a89 Compare July 18, 2023 10:32
@blushi
Copy link
Member

blushi commented Jul 18, 2023

With a fresh regen-web install, I can't reproduce the storybook dev server issue locally as described in https://www.notion.so/regennetwork/Engineering-Architecture-2023-07-06-c686348bcb2941ee8c0961a34589f3d2
Maybe some local cache issue that @flagrede had? storybookjs/storybook#22808 (comment)
So I've just rebased dev and requesting reviews on this, it would be good to test the storybook dev server works for you too @wgwz @ryanchristo

@blushi blushi force-pushed the chore-1316-vite-migration branch from 406b229 to 0a83d69 Compare July 18, 2023 11:45
vitePluginRequire(),
visualizer(),
],
define: isDev ? { global: {} } : { 'process.env': {} },
Copy link
Member

@blushi blushi Jul 18, 2023

Choose a reason for hiding this comment

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

see vitejs/vite#1973
we were getting errors when trying to login because @keplr-wallet/stores uses process.env internally

Copy link
Contributor

Choose a reason for hiding this comment

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

why would they not support process.env? 🤦

@blushi blushi requested a review from a team July 18, 2023 12:04
@blushi
Copy link
Member

blushi commented Jul 18, 2023

@erikalogie could you test that everything is working as expected on https://deploy-preview-1955--regen-registry.netlify.app/? Thanks!

@erikalogie
Copy link
Collaborator

Ok @blushi this is what I found so far but I'm going to keep testing:

This is a bit weird but is this just a metadata issue? https://monosnap.com/file/SrdM9rP9Dp5X4hFo6y59dQtc0Hn376

The impact cards width is not correct on project pages, should be as wide as the column and the OTC card below: https://monosnap.com/file/t5ZAGdCzMxq1LAh0WdcSpYkWeBHem2

No map is showing up at the top of some of the project pages: https://deploy-preview-1955--regen-registry.netlify.app/project/C02-016

or for example here there is a location but the map is blank: https://monosnap.com/file/sWiau7inTsnbqJxMqv0HvzhWSTLpb5

@ryanchristo
Copy link
Member

I'm receiving the following error on the storybook dev server:

ReferenceError: require is not defined

Screenshot from 2023-07-18 08-14-11

@blushi
Copy link
Member

blushi commented Jul 18, 2023

I'm receiving the following error on the storybook dev server:

ReferenceError: require is not defined

Screenshot from 2023-07-18 08-14-11

have you rebuilt the web-components? (by rerunning for instance yarn watch?)
I would also suggest to rm all your node_modules and re-install

@wgwz
Copy link
Contributor

wgwz commented Jul 18, 2023

With a fresh regen-web install, I can't reproduce the storybook dev server issue locally as described in https://www.notion.so/regennetwork/Engineering-Architecture-2023-07-06-c686348bcb2941ee8c0961a34589f3d2 Maybe some local cache issue? storybookjs/storybook#22808 (comment) So I've just rebased dev and requesting reviews on this, it would be good to test the storybook dev server works for you too

@blushi i used a fresh clone of the regen-web repo just in case, but even after doing that and running this sequence of commands i'm seeing some errors:

Commands:

$ yarn
$ yarn watch
$ yarn storybook

I.e. this storybook component works ok:
Screen Shot 2023-07-18 at 11 24 33 AM

but then for "tx successful modal" i'm seeing this:

Screen Shot 2023-07-18 at 11 25 53 AM

@wgwz
Copy link
Contributor

wgwz commented Jul 18, 2023

Similarly for the image storybook component i'm seeing:

Screen Shot 2023-07-18 at 11 34 36 AM

But the "action" component that @ryanchristo was having an issue with seems to be working OK for me:

Screen Shot 2023-07-18 at 11 36 34 AM

Copy link
Member

@ryanchristo ryanchristo left a comment

Choose a reason for hiding this comment

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

tACK

The issues erika identified seem to be unrelated to the migration itself so we should track those items separately if we feel they need to be addressed.

I got storybook running in my local environment and am now seeing the same errors kyle identified but the image error seems to be working on the deploy preview.

If we can resolve the undefined buffer errors in the modal section, that would be the one new issue I see, but if this is blocking regen-network/rnd-dev-team#1537 and the solution is not straightforward, I'm ok with addressing in a followup.

@blushi
Copy link
Member

blushi commented Jul 19, 2023

This is a bit weird but is this just a metadata issue? https://monosnap.com/file/SrdM9rP9Dp5X4hFo6y59dQtc0Hn376

This is on dev too, somehow the story is in the anchored metadata for this project (it should only be in the unanchored data), I suspect this might have occurred while developing/testing and is not related to the changes here.

The impact cards width is not correct on project pages, should be as wide as the column and the OTC card below: https://monosnap.com/file/t5ZAGdCzMxq1LAh0WdcSpYkWeBHem2

This is on dev too, so probably we should open up a separate issue to fix this.

No map is showing up at the top of some of the project pages: https://deploy-preview-1955--regen-registry.netlify.app/project/C02-016

or for example here there is a location but the map is blank: https://monosnap.com/file/sWiau7inTsnbqJxMqv0HvzhWSTLpb5

This was because we were missing the mapbox token in the env variables (since migrating to vite made us change the env variables names), it's fixed now.

@blushi
Copy link
Member

blushi commented Jul 19, 2023

Similarly for the image storybook component i'm seeing:

Screen Shot 2023-07-18 at 11 34 36 AM

We were still using process.env in our storybook stories so I updated this as well to use import.meta.env instead...

@blushi
Copy link
Member

blushi commented Jul 19, 2023

If we can resolve the undefined buffer errors in the modal section, that would be the one new issue I see, but if this is blocking regen-network/regen-registry#1537 and the solution is not straightforward, I'm ok with addressing in a followup.

Found a workaround in the mean time but we can definitely fix this in a separate issue. See my comment in 3a4e765#diff-575d6b1c6b6f565f9b9037a316bbd68ab89898eeeab004bff7d6e79e0e512860R177-R179

@erikalogie
Copy link
Collaborator

erikalogie commented Jul 19, 2023

This is a bit weird but is this just a metadata issue? https://monosnap.com/file/SrdM9rP9Dp5X4hFo6y59dQtc0Hn376

This is on dev too, somehow the story is in the anchored metadata for this project (it should only be in the unanchored data), I suspect this might have occurred while developing/testing and is not related to the changes here.

Ok, will ignore for now.

The impact cards width is not correct on project pages, should be as wide as the column and the OTC card below: https://monosnap.com/file/t5ZAGdCzMxq1LAh0WdcSpYkWeBHem2

This is on dev too, so probably we should open up a separate issue to fix this.

Ok, I opened a task for this https://app.zenhub.com/workspaces/rnd-dev-team-5f8998bec8958d000f4609e2/issues/gh/regen-network/regen-registry/1762

@blushi blushi merged commit a23fab8 into dev Jul 20, 2023
14 checks passed
@blushi blushi deleted the chore-1316-vite-migration branch July 20, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate from CRA to Vite
5 participants