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

fix: do not derive cache key if component is static #630

Merged
merged 1 commit into from
Oct 12, 2020

Conversation

theKashey
Copy link
Collaborator

Summary

For components, which are static, there is no reason to derive cache key from props.

👉 Do not derive cache key if component is static, fixes #629

@diffidentDude
Copy link

Nice, this looks like it would fix the issue we were having, is there an alpha to test with?

@theKashey
Copy link
Collaborator Author

@diffidentDude - any change that you can replace null -> "static" directly in your node_modules to double check that it works fine for you?

@diffidentDude
Copy link

@diffidentDude - any change that you can replace null -> "static" directly in your node_modules to double check that it works fine for you?

Sure, can do.

@caizx666
Copy link

TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
| property 'backendConnector' -> object with constructor 't'
| property 'backend' -> object with constructor 'e'
--- property 'services' closes the circle
at JSON.stringify ()
at o.a.getCacheKey (loadable.esm.js:227)
at o.a.getCache (loadable.esm.js:235)
at o.a.componentDidMount (loadable.esm.js:192)
at Sr (react-dom.production.min.js:212)
at oi (react-dom.production.min.js:255)
at t.unstable_runWithPriority (scheduler.production.min.js:19)
at Ut (react-dom.production.min.js:122)
at ii (react-dom.production.min.js:248)
at Ur (react-dom.production.min.js:239)

@theKashey
Copy link
Collaborator Author

@caizx666 - you mean #629, this is the fix for your problem.

@caizx666
Copy link

Yes

@kuoruan
Copy link
Contributor

kuoruan commented Sep 23, 2020

Any update for this commit?

@theKashey
Copy link
Collaborator Author

oof, a week already passed. If someone can confirm (@diffidentDude 😉) that is solving the problem, and not creating a new one - I can release the update immediately.

I just not quite trust build in unit tests and had no free time to test stuff manually.

@theKashey theKashey merged commit b4151d8 into master Oct 12, 2020
@bevacqua
Copy link

This was merged a week ago but it hasn't been published as a new npm package version yet. Can we get 5.13.3? 🙇

@theKashey
Copy link
Collaborator Author

Just double checking webpack5 before the new release...

fivethreeo pushed a commit to fivethreeo/loadable-components that referenced this pull request Nov 16, 2022
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.

Error: Converting circular structure to JSON
5 participants