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

🔧 POW Server side rendering #7725

Closed
wants to merge 7 commits into from
Closed

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Oct 18, 2023

Let me know your thoughts about this change.

SSR is needed if we want to use all Nuxt power

@netlify
Copy link

netlify bot commented Oct 18, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit d2a3520
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/6552186ee7f8e400082eeb93
😎 Deploy Preview https://deploy-preview-7725--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
Copy link
Contributor

reviewpad bot commented Oct 18, 2023

AI-Generated Summary: This pull request makes extensive changes across multiple files focusing primarily on preventing potential errors during server-side rendering. It introduces checks using process.client to best ensure that specific 'window' and 'localStorage' operations are only executed on the client-side where they are available. This adjustment allows for more reliable performance in both client and server-side environments and is likely guided towards improving compatibility with server-side rendering (SSR) and enhancing Universal Render environments.

External package imports have been modified, such as @polkadot/extension-dapp and @ramp-network/ramp-instant-sdk, often being adjusted to only import when needed. In some components, these are moved from a static import at the start of the file to a dynamic import within a specific function. Various parts of the script are also now wrapped with Vue's client-only component to preserve intended functionality while avoiding potential errors during server-side rendering. This suggests an intent to reduce package load times and potentially improve overall performance.

Some Vue components saw changes to their lifecycle hooks, with moves from the use of 'onBeforeUnmount' hook to 'onMounted' hook for event listeners, specifying essential processes to only run after the component has transitioned into the DOM.

Lastly, changes to the project package.json and pnpm-lock.yaml files denote alterations in dependency management, including the movement of @ramp-network/ramp-instant-sdk and the addition of ts-invariant version 0.10.3.

Overall, these changes aim to improve flexibility and compatibility across various environments, potentially enhancing performance, load times, and error handling in both client and server-side contexts.

@reviewpad reviewpad bot added the large Pull request is large label Oct 18, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Oct 18, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@roiLeo
Copy link
Contributor Author

roiLeo commented Oct 18, 2023

😢
Screenshot 2023-10-18 at 11-50-40 Invocation Failed

@preschian
Copy link
Member

as long the build and deployed successfull im fine with it 👍🏻

@roiLeo
Copy link
Contributor Author

roiLeo commented Oct 18, 2023

So many things to reconfigure on client-side, don't mind first load only showing footer it's normal.
Looks like all playwright tests are broken now 😞

@roiLeo roiLeo marked this pull request as ready for review October 18, 2023 12:03
@roiLeo roiLeo requested a review from a team as a code owner October 18, 2023 12:03
@roiLeo roiLeo marked this pull request as draft October 18, 2023 12:30
@yangwao
Copy link
Member

yangwao commented Oct 20, 2023

What can potentially break?

@roiLeo
Copy link
Contributor Author

roiLeo commented Oct 20, 2023

What can potentially break?

UI/UX, client side plugin and maybe more?

@yangwao
Copy link
Member

yangwao commented Oct 25, 2023

UI/UX, client side plugin and maybe more?

And do we need to setup something special from CF/Netlify to work?

@roiLeo
Copy link
Contributor Author

roiLeo commented Oct 25, 2023

And do we need to setup something special from CF/Netlify to work?

Nope, only check which components needs to be loaded on client-side and which can be loaded on server side.
For now I've wrapped most components on client-side that why we have a "delayed loading" and weird darkmode

note: SSR is needed for SEO & prerender stuff

@roiLeo
Copy link
Contributor Author

roiLeo commented Nov 2, 2023

// TODO: persist localstorage (or maybe vueuse equivalent)

@roiLeo roiLeo mentioned this pull request Nov 3, 2023
6 tasks
@yangwao yangwao mentioned this pull request Nov 13, 2023
1 task
Copy link

codeclimate bot commented Nov 13, 2023

Code Climate has analyzed commit d2a3520 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2

View more on Code Climate.

Copy link

sonarcloud bot commented Nov 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.2% 0.2% Duplication

@shashkovdanil shashkovdanil mentioned this pull request Dec 2, 2023
16 tasks
@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 8, 2023

more changes in #8356

@roiLeo roiLeo closed this Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants