Update dependencies to work with Vite and NextJS, combine css files into a single css file #1510

vladimir-mck · 2023-08-30T12:04:04Z

Description

Development notes

This pull request addresses several JavaScript issues, enhancing the compatibility of Kedro-Viz when integrated with Svelte and Next.js. Although the JS-related errors have been resolved.

Replaced lodash. dependencies with lodash to ensure compatibility with both Next.js and Vite.
Upgraded Plotly to a newer version to fix errors experienced in both Vite and Next.js environments.
Removed css imports from components and replaced with scsc imports
Remove script to transform scss into css in dev mode
Added script to combine lib css files into a single css file

Vite:

QA notes

Create a new project with Vite:

npm create vite@latest my-react-app --template react
install kedro-viz alpha version npm i @quantumblack/[email protected]
add the component and styles
data to it.

Create a new project with NextJS:

npx create-next-app@latest
install kedro-viz alpha version npm i @quantumblack/[email protected]
Import kedro following update readme

Checklist

Read the contributing guidelines
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added new entries to the RELEASE.md file
Added tests to cover my changes

ravi-kumar-pilla

Nice work Vlad !!

Some questions -

I want to understand why we need apollo config refactoring ?
I have tested the PR using the QA notes, the alpha release works well except for the CSS errors (as you mentioned). Just to confirm, I am attaching the errors during the run

Svelte error -

Next.js error -

vladimir-mck · 2023-09-01T08:09:19Z

Hey Ravi

I want to understand why we need apollo config refactoring ?

So here we needed a bit of a refactor because it tries to access the window.location object to get the host, path name and protocol. The window object doesn't exist when it's rendered on the server and it's throwing an error, additionally the web socket link will also throw an error.

I have tested the PR using the QA notes, the alpha release works well except for the CSS errors (as you mentioned). Just to confirm, I am attaching the errors during the run

Yep, that's okay. The css error is not present when started with Vite. Next has it's own rules how to handle css and that's why it's throwing an error. I have to agree with them on this, we shouldn't release with 66 css files, one is enough.

…k worker for SSR Signed-off-by: Vladimir <[email protected]>

* Fix: Adding favicon to Kedro Demo * Fix: Change in approach for serving favicon * Lint error fix * Lint error fix * Favicon endpoint test added * Favicon endpoint test added * Lint error fixed * Fix: Adding favicon to Kedro Demo Signed-off-by: Jitendra Gundaniya <[email protected]> * Fix: Change in approach for serving favicon Signed-off-by: Jitendra Gundaniya <[email protected]> * Lint error fix Signed-off-by: Jitendra Gundaniya <[email protected]> * Lint error fix Signed-off-by: Jitendra Gundaniya <[email protected]> * Favicon endpoint test added Signed-off-by: Jitendra Gundaniya <[email protected]> * Favicon endpoint test added Signed-off-by: Jitendra Gundaniya <[email protected]> * Lint error fixed Signed-off-by: Jitendra Gundaniya <[email protected]> * Fixed favicon endpoint test * Release doc updated * Update RELEASE.md Co-authored-by: rashidakanchwala <[email protected]> * Removed pytest.fixture as per review comment --------- Signed-off-by: Jitendra Gundaniya <[email protected]> Co-authored-by: rashidakanchwala <[email protected]> Signed-off-by: Vladimir <[email protected]>

* v6.5.0 * release * update-reminder-content * update reminder Signed-off-by: Vladimir <[email protected]>

Signed-off-by: Vladimir <[email protected]>

… update babel to remove scss imports from the lib components. Revert apollo config and worker Signed-off-by: Vladimir <[email protected]>

Signed-off-by: Vladimir <[email protected]>

.github/workflows/check-release.yml

Signed-off-by: Vladimir <[email protected]>

tynandebold

The code is looking fantastic. I'm excited for this change to go in.

One thing when I'm trying to test is I can't install the alpha version of the package. I get this error:

Do you see the same?

Signed-off-by: Vladimir <[email protected]>

tynandebold

I tried this out using create react app and everything loads and works fine. I see that some of the styles are off though. Have a look:

This is the alpha version I'm using: "@quantumblack/kedro-viz": "^6.5.1-alpha.2".

…ports Signed-off-by: Vladimir <[email protected]>

tynandebold

Really nice! Everything is looking great and working well.

I've left a couple small comments, nothing blocking.

tynandebold · 2023-09-12T17:33:07Z

RELEASE.md

@@ -17,7 +23,7 @@ Please follow the established format:
 - Fix to search for a '<lambda' Python function in the sidebar. (#1497)
 - Add favicon to Kedro-Viz. (#1509)
 - Remove python upper-bound requirements and add KedroVizPythonVersion warning. (#1506)
-
+- Resolved various bugs affecting server-side rendering, updated dependencies to ensure compatibility with Vite and Next.js environments (#1508)


Why is this here? That PR was never merged, right?

RELEASE.md

ravi-kumar-pilla · 2023-09-12T18:16:21Z

README.md

+
+const NoSSRKedro = dynamic(() => import('@quantumblack/kedro-viz'), {
+  ssr: false,
+});


ravi-kumar-pilla

Awesome work Vlad 👍 . Need some changes with the release doc but nothing blocking.

Co-authored-by: Tynan DeBold <[email protected]>

vladimir-mck requested a review from tynandebold as a code owner August 30, 2023 12:04

vladimir-mck self-assigned this Aug 30, 2023

vladimir-mck requested a review from yetudada as a code owner August 30, 2023 12:04

vladimir-mck changed the title ~~Update dependencies, update apollo config, update worker to use a moc…~~ Fix server side rendering bugs, update dependencies to work with Vite and NextJS Aug 30, 2023

vladimir-mck requested review from jitu5 and ravi-kumar-pilla August 30, 2023 12:07

vladimir-mck force-pushed the bugfix/1293-1504 branch from e5d54a2 to 18225d2 Compare August 30, 2023 12:07

ravi-kumar-pilla approved these changes Aug 31, 2023

View reviewed changes

vladimir-mck changed the title ~~Fix server side rendering bugs, update dependencies to work with Vite and NextJS~~ Update dependencies to work with Vite and NextJS, combine css files into a single css file Sep 6, 2023

vladimir-mck and others added 8 commits September 6, 2023 13:02

Update dependencies, update apollo config, update worker to use a moc…

1e96cb8

…k worker for SSR Signed-off-by: Vladimir <[email protected]>

Release v6.5.0 (#1513)

8791522

* v6.5.0 * release * update-reminder-content * update reminder Signed-off-by: Vladimir <[email protected]>

remove branch condition for automate release version check (#1514)

61c1aef

Signed-off-by: Vladimir <[email protected]>

Update RELEASE.md

d335953

Signed-off-by: Vladimir <[email protected]>

Update css imports to scss, combine css files into 1 for the npm lib,…

3e13110

… update babel to remove scss imports from the lib components. Revert apollo config and worker Signed-off-by: Vladimir <[email protected]>

Update package lock

68162fb

Signed-off-by: Vladimir <[email protected]>

Update readme and release me

2e03657

Signed-off-by: Vladimir <[email protected]>

vladimir-mck force-pushed the bugfix/1293-1504 branch from e5a49ee to 2e03657 Compare September 6, 2023 11:05

vladimir-mck requested a review from rashidakanchwala as a code owner September 6, 2023 11:05

vladimir-mck commented Sep 6, 2023

View reviewed changes

.github/workflows/check-release.yml Outdated Show resolved Hide resolved

vladimir-mck requested a review from ravi-kumar-pilla September 6, 2023 11:14

vladimir-mck added 2 commits September 6, 2023 13:17

Merge branch 'main' into bugfix/1293-1504

a31102d

Remove build:css script from circle ci config

4454602

Signed-off-by: Vladimir <[email protected]>

tynandebold reviewed Sep 7, 2023

View reviewed changes

vladimir-mck added 2 commits September 8, 2023 09:59

Bundle css into lib/styles folder

4366c26

Signed-off-by: Vladimir <[email protected]>

Update readme

c3b3568

Signed-off-by: Vladimir <[email protected]>

tynandebold reviewed Sep 8, 2023

View reviewed changes

Add webpack config to extract scss and babel plugin to remove scss im…

3882307

…ports Signed-off-by: Vladimir <[email protected]>

vladimir-mck requested a review from tynandebold September 12, 2023 09:39

tynandebold approved these changes Sep 12, 2023

View reviewed changes

tynandebold removed the request for review from yetudada September 12, 2023 17:42

Merge branch 'main' into bugfix/1293-1504

d1c635a

ravi-kumar-pilla reviewed Sep 12, 2023

View reviewed changes

README.md

const NoSSRKedro = dynamic(() => import('@quantumblack/kedro-viz'), {

ssr: false,

});

Copy link

Contributor

ravi-kumar-pilla Sep 12, 2023

Choose a reason for hiding this comment

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

Awesome !!

ravi-kumar-pilla approved these changes Sep 12, 2023

View reviewed changes

vladimir-mck and others added 2 commits September 13, 2023 09:24

Update RELEASE.md

3156597

Co-authored-by: Tynan DeBold <[email protected]>

Update RELEASE.md

84e9e0e

vladimir-mck merged commit 2d0c577 into main Sep 13, 2023
1 check passed

vladimir-mck deleted the bugfix/1293-1504 branch September 13, 2023 09:35

tynandebold mentioned this pull request Oct 10, 2023

Release v6.6.0 #1566

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies to work with Vite and NextJS, combine css files into a single css file #1510

Update dependencies to work with Vite and NextJS, combine css files into a single css file #1510

vladimir-mck commented Aug 30, 2023 •

edited

Loading

ravi-kumar-pilla left a comment

vladimir-mck commented Sep 1, 2023 •

edited

Loading

tynandebold left a comment

tynandebold left a comment

tynandebold left a comment

tynandebold Sep 12, 2023

ravi-kumar-pilla Sep 12, 2023

ravi-kumar-pilla Sep 12, 2023

ravi-kumar-pilla left a comment

Update dependencies to work with Vite and NextJS, combine css files into a single css file #1510

Update dependencies to work with Vite and NextJS, combine css files into a single css file #1510

Conversation

vladimir-mck commented Aug 30, 2023 • edited Loading

Description

Development notes

QA notes

Checklist

ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

vladimir-mck commented Sep 1, 2023 • edited Loading

tynandebold left a comment

Choose a reason for hiding this comment

tynandebold left a comment

Choose a reason for hiding this comment

tynandebold left a comment

Choose a reason for hiding this comment

tynandebold Sep 12, 2023

Choose a reason for hiding this comment

ravi-kumar-pilla Sep 12, 2023

Choose a reason for hiding this comment

ravi-kumar-pilla Sep 12, 2023

Choose a reason for hiding this comment

ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

vladimir-mck commented Aug 30, 2023 •

edited

Loading

vladimir-mck commented Sep 1, 2023 •

edited

Loading