packages
packages copied to clipboard
[webview_flutter_wkwebview] Fixes JSON.stringify() cannot serialize cyclic structures
Using the replacer
parameter of JSON.stringify()
to remove cyclic object to resolve the following error.
TypeError: JSON.stringify cannot serialize cyclic structures.
~~Related solution: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value~~
Related issue: https://github.com/flutter/flutter/issues/144535
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format
.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences]
- [x] I linked to at least one issue that this PR fixes in the description above.
- [x] I updated
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.md
to add a description of the change, following repository CHANGELOG style. - [x] I updated/added relevant documentation (doc comments with
///
). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
This is in wkwebview but maybe @ditman could review the js code?
Can do, thanks for the mention @jmagman, I normally disregard webview stuff because I end up flagged as an owner (but I'm not :P)
Ahem:
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value#circular_references
The biggest problem I see with this is that this is a "lossy" fix; what you deserialize is not what you were serializing; "[Cyclic]"
might be good for the json encoding to not crash, but it doesn't tell you how to recover that cycle when you parse JSON back to an object?
(Also if the cycle is not important: maybe remove it before attempting to serialize to JSON?)
((PS: I don't think I've had to deal with JSON malformed like this before, but maybe with something like GraphQL you could end up with a circular dependency on your data, not sure...))
The biggest problem I see with this is that this is a "lossy" fix; what you deserialize is not what you were serializing;
"[Cyclic]"
might be good for the json encoding to not crash, but it doesn't tell you how to recover that cycle when you parse JSON back to an object?(Also if the cycle is not important: maybe remove it before attempting to serialize to JSON?)
Yea, I was considering this as well. But do think it is reasonable for a user to expect that console.log
should be able to handle any object? Does the problem lie with us using JSON.stringify()
instead of just using String()
? I would consider calling console.log
as the equivalent of calling print(...)
in a Flutter app and would be surprised if this caused an error on recursion. It might be a breaking change to move away from JSON.stringify()
now though, so my reasoning was to avoid the crash as the best solution right now.
Looking at the issue and the Android output, it looks like Android basically just calls String() on an object passed to the console. Android just returns [object Object]
when passed a recursive object.
Ah! I didn't realize this was to implement console.log
. In that case, this is probably fine.
The way this works in the browser is by it not attempting to "toString" the whole thing, it just gives you a tree-like structure for the object:
(You get tired of expanding self
before the browser runs out of memory, I think :P)
The PR is a C+P from MDN however; @stuartmorgan licensing concerns?
The PR is a C+P from MDN however; @stuartmorgan licensing concerns?
What is done in this PR is not permitted in Flutter.
This code is not complicated enough to warrant us adding a third_party
directory to this plugin; we should get a clean implementation of the webkit_webview_controller.dart change (by giving someone who has not seen this code just the unit test and a high-level description of the goal).
@stuartmorgan Thanks for your reminder.
I have adjusted the implementation to solve this issue by removing the cyclic object, please review the code again.
Unless I'm missing something, the new version is destroying any duplicate objects, which is not the same thing as just removing cycles.
@stuartmorgan I've re-adjusted the removal logic.
cc ~~@hellohuanlin~~ @cbracken to review