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

🔧 wip: nuxt3 #6917

Closed
wants to merge 27 commits into from
Closed

🔧 wip: nuxt3 #6917

wants to merge 27 commits into from

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Aug 23, 2023

@kodadot/code-review-guild I'm stuck on some build errors if anyone want to try out feel free to checkout this branch

  • Goal is to display simple hello world on Landing

Screenshot 2023-08-23 at 15-59-18 🔧 wip nuxt3 by roiLeo · Pull Request #6917 · kodadot_nft-gallery
jerry

@roiLeo roiLeo requested a review from a team as a code owner August 23, 2023 13:55
@roiLeo roiLeo requested review from vikiival and Jarsen136 and removed request for a team August 23, 2023 13:55
@roiLeo roiLeo marked this pull request as draft August 23, 2023 13:55
@netlify
Copy link

netlify bot commented Aug 23, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 392bf0b
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64ee0e3b37b65e00083d8132
😎 Deploy Preview https://deploy-preview-6917--koda-canary.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.

@reviewpad reviewpad bot added the large Pull request is large label Aug 23, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Aug 23, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 17895 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 27387 lines exceeds the maximum allowed for the inline comments feature.

@socket-security
Copy link

socket-security bot commented Aug 23, 2023

New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
rollup 2.0.0 None +0 2.65 MB lukastaegert
vite 3.2.7...4.2.0 environment +4/-4 3.78 MB vitebot
histoire 0.16.4...0.16.2 None +5/-7 5.86 MB akryum
vitest 0.34.1...0.34.2 None +10/-12 11.1 MB oreanno

🚮 Removed packages: @nuxt/[email protected], [email protected], [email protected]

@socket-security
Copy link

socket-security bot commented Aug 23, 2023

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

@yangwao
Copy link
Member

yangwao commented Aug 23, 2023

Hey, is insider trading allowed, what do you think
https://app.zeitgeist.pm/markets/238

@yangwao
Copy link
Member

yangwao commented Aug 24, 2023

maybe @stephenjason89 😄

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 28833 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24513 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24541 lines exceeds the maximum allowed for the inline comments feature.

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 24, 2023

Screenshot 2023-08-24 at 16-37-19 https __deploy-preview-6917--koda-canary netlify app

Now it builds 🎊

excited-happy

@stephenjason89
Copy link
Contributor

Good job @roiLeo, It's 12midnight here. If you need help just let me know. My tg handle is SOW89

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 24077 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 20746 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 20768 lines exceeds the maximum allowed for the inline comments feature.

@yangwao
Copy link
Member

yangwao commented Aug 29, 2023

What's the strategy for merging this?

You want to do page by page or do some nuxt layout?

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 29, 2023

What's the strategy for merging this?

Wait for some issue to be resolved, I can handle conflicts & push from others (fork my repo)

You want to do page by page or do some nuxt layout?

  • fix all gql calls (some of theme have been fixed) page by page & components by components

From #2690 (comment)

I've done:

  • upgrade to nuxt3, remove nuxt-bridge & edge
  • upgrade i18
  • upgrade @nuxtjs/apollo
  • update layouts & page with vue3 composition (<NuxtLayout><NuxtPage /></NuxtLayout>)
  • rename /static folder to /public

We need to fix some packages config now.

@stephenjason89 have you made some progress on I18n plugin?

@preschian
Copy link
Member

@roiLeo, it seems like we can use a proxy to gradually release it to the user page by page. Maybe we can start with /blog by implementing @nuxt/content? It would be great if we could support SSR also for /blog. So, we can drop prerender.io for /blog

fix all gql calls (some of theme have been fixed) page by page & components by components

I think this is the major one

#6277 : major need

this one can't be solved with just a fork?

@preschian
Copy link
Member

@roiLeo, but if you prefer not to use a proxy, we can stick with this way

rename /static folder to /public

Are there any changes that need to merge in main to reduce the diff? I guess that renaming /static to /public can be made in the main branch

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 29, 2023

@roiLeo, it seems like we can use a proxy to gradually release it to the user page by page. Maybe we can start with /blog by implementing @nuxt/content? It would be great if we could support SSR also for /blog. So, we can drop prerender.io for /blog

nuxt/content doesn't work on ssr: false, SSR support will be a major update, I don't think it's a good idea to realease first version of nuxt3 with it.

#6277 : major need

this one can't be solved with just a fork?

wdym? I want to use it like it's on Doc (const { oruga } = useProgrammatic()) I think we can extend it (const { neo } = useProgrammatic())

@roiLeo, but if you prefer not to use a proxy

I'd rather not

rename /static folder to /public

Are there any changes that need to merge in main to reduce the diff? I guess that renaming /static to /public can be made in the main branch

Idk if public folder is working with nuxt-bridge

@preschian
Copy link
Member

preschian commented Aug 29, 2023

wdym? I want to use it like it's on Doc (const { oruga } = useProgrammatic()) I think we can extend it (const { neo } = useProgrammatic())

You need to fork this script first https://github.com/oruga-ui/oruga/blob/4915c4dbcb1a6c2cd39bc7660d41916b330bc0cd/packages/oruga-next/src/components/modal/index.ts#L8. actually similar with what I do in this PR https://github.com/kodadot/nft-gallery/pull/6714/files#diff-a59a359d665e8dbbfbd08707a6b82aab528e21e3debcfc707f81d7f505015073R9

what I do in that PR:

  1. I fork their implementation and put it here libs/ui/src/components/NeoModalExtend/plugin.js
  2. inject it in our plugin here plugins/oruga-modal.ts

and then can create or inject the composables through a plugin. this is how they install it to the plugin:

I can help with this one once I'm done with a simple mint

@yangwao
Copy link
Member

yangwao commented Aug 29, 2023

Wait for some issue to be resolved

So I can create a new chief issue for migration and replace this one?

To not break our current flow and keep a flawless release cycle, I would propose making alpha branch with a deployment.

Later we can still back merge new things and once it is even 1:1 alpha with main we can update.

lmk and I can proceed.

Hanging out in one PR for this major upgrade is not so good I think.

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 29, 2023

I can help with this one once I'm done with a simple mint

Yes please! could you push your code to my repo?

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 20806 lines exceeds the maximum allowed for the inline comments feature.

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 29, 2023

So I can create a new chief issue for migration and replace this one?

To not break our current flow and keep a flawless release cycle, I would propose making alpha branch with a deployment.

IMO, it's better that someone (Kodadev insider) handle the migration, it would be less exhausting than having to go through multiple reviews and better to resolve conflicts.

Later we can still back merge new things and once it is even 1:1 alpha with main we can update.

Yes! Once we fix oruga (programmatic) & apollo config, we could be able to release an alpha branch.

Hanging out in one PR for this major upgrade is not so good I think.

why that?

note: not happening before 01/09 sorry

@daiagi
Copy link
Contributor

daiagi commented Aug 29, 2023

@roiLeo
I'm lost in the discussion
is there anything concrete I can help you with right now?

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 29, 2023

@roiLeo I'm lost in the discussion is there anything concrete I can help you with right now?

Would you like to check/fix a component, a page or a plugin? Just let me know wich part you want to work on.
For now preschian want to work on programmatic modal (oruga modal) & stephen is on i18n config.

@daiagi
Copy link
Contributor

daiagi commented Aug 29, 2023

component/page please

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 29, 2023

component/page please

Try first with something easy/doable like spotlight, series-insight, assets page then push on my repo and I'll merge it as quickly as I can (if it's working)

edit:

📔 To anyone going to work with page/component that use $apollo here's how to replace it:

  • BASIC:
const { result: res } = useQuery(query, variables)
  • CALLBACK
const res = ref()
const { onResult } = useQuery(query, variables)
onResult((result) => (res.value = result.data))
  • ASYNC f°:
const res = ref()
const fetchData = async () => {
  const { data } = await useAsyncQuery(query, { ...variables })
  res.value = data
}

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 20842 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Aug 29, 2023

Code Climate has analyzed commit 392bf0b and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Aug 29, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 35 Code Smells

No Coverage information No Coverage information
0.6% 0.6% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@yangwao yangwao mentioned this pull request Aug 29, 2023
18 tasks
@yangwao
Copy link
Member

yangwao commented Aug 29, 2023

IMO, it's better that someone (Kodadev insider) handle the migration, it would be less exhausting than having to go through multiple reviews and better to resolve conflicts.

I've made nuxt branch, deployment is working on https://nuxt.kodadot.xyz

  • @roiLeo please merge it there what you have and we can split load. Thank you for understanding.

why that?

This

image

and because the process is not around the KodaDot branch in this repository and we can split load among others, not make you block migration by single dev on resolving conflicts. Hopefully, you understand.

Rather encourage split tasks in

For others likewise, making it against nuxt branch.

@roiLeo roiLeo mentioned this pull request Aug 30, 2023
@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 30, 2023

@roiLeo roiLeo closed this Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-nuxt A-vue3 large Pull request is large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants