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

Temporary fix for graphNew error in local development #2006

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

ravi-kumar-pilla
Copy link
Contributor

@ravi-kumar-pilla ravi-kumar-pilla commented Jul 28, 2024

Description

Partially resolves #1913

Fixes the below error with a temporary work-around:

graph.js:36 Uncaught (in promise) TypeError: instance.graphNew is not a function
    at layout (graph.js:36:1)
    at worker.js:58:1
    at graph.js:53:1
    at Object.dispatch (index.js:16:1)
    at Object.dispatch (<anonymous>:6:7384)
    at store.js:19:1
    at index.js:17:1
    at Object.dispatch (redux.js:297:1)
    at s (<anonymous>:3:5093)
    at index.js:20:1

Development notes

There are some compatibility issues with workerize-loader and Webpack v5 which is affecting the local dev env. As @jitu5 and I discussed, this issue happened mostly after node18 upgrade. As Jitendra pointed out to these commits

Also a similar issue - developit/workerize-loader#77 , for which some are suggesting having a simple own library. Since this issue happens in dev env, I felt we opt for a less effort work-around. Further looking at the fix, I realized this might be an issue with cache.

What does this PR do -

Introduces a script - "start:dev": "rm -rf node_modules/.cache && npm start", at package.json which can be used in local development instead of doing npm start. Now developers can make use of this script by running npm run start:dev which helps removing the cache and resolving the error.

QA notes

  • npm run start:dev in local should not throw the above error
  • All CI tests should pass

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 ravi-kumar-pilla changed the title Temporary Fix for graphNew Error in local development Temporary fix for graphNew error in local development Jul 28, 2024
@ravi-kumar-pilla ravi-kumar-pilla marked this pull request as ready for review July 29, 2024 13:43
@ravi-kumar-pilla ravi-kumar-pilla marked this pull request as draft July 29, 2024 13:43
@ravi-kumar-pilla ravi-kumar-pilla marked this pull request as ready for review July 29, 2024 13:48
Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

I have been doing this morning. So useful. thanks @ravi-kumar-pilla

Copy link
Contributor

@jitu5 jitu5 left a comment

Choose a reason for hiding this comment

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

Thanks Ravi.

@ravi-kumar-pilla ravi-kumar-pilla merged commit e719f30 into main Jul 29, 2024
19 checks passed
@ravi-kumar-pilla ravi-kumar-pilla deleted the fix/temp-graphNew branch July 29, 2024 19:04
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.

On kedro-viz UI startup error: instance.graphNew is not a function
3 participants