Weird naming collision in @urql/exchange-graphcache crashes in react-native

[edenstrom]

Hey,

So we've noticed that urql offline exchange doesn't work in the release builds of our react native app. It works fine in Debug mode (I guess because of the different engines)

Could be something with our setup, but the following is what I've found.

urql version & exchanges:
"@urql/core": "^2.0.0",
"@urql/devtools": "^2.0.3",
"@urql/exchange-auth": "^0.1.2",
"@urql/exchange-graphcache": "^4.0.0",
"@urql/exchange-retry": "^0.2.0",
"urql": "^2.0.2",

Steps to reproduce

1. Try to use offline exchange in a react native app.

Below is a code snippet from https://unpkg.com/@urql/[email protected]/dist/urql-exchange-graphcache.js

You can also search for it with function a(a) and you'll find it in the function persistData.

}, persistData = function() {
  if (currentData.storage) {
    currentOptimistic = !0;
    currentOperation = "read";
    var b = makeDict();
    currentData.persist.forEach((function a(a) {
      var f, c = deserializeKeyInfo(a), d = c.entityKey;
      void 0 !== (f = readLink(d, c = c.fieldKey)) ? b[a] = ":" + core.stringifyVariables(f) : void 0 !== (f = readRecord(d, c)) ? b[a] = core.stringifyVariables(f) : b[a] = void 0;
    }));
    currentOptimistic = !1;
    currentData.storage.writeData(b);
    currentData.persist.clear();
  }
}, hydrateData = function(a, b, c) {

2. Look at the line currentData.persist.forEach((function a(a) {. Both the function and the parameter is a.
3. The next line is deserializeKeyInfo(a) which sends a as an argument into the deserializeKeyInfo.
4. In debug mode, a is the variable a (parameter to the function in step 2).
5. In release mode, a is the function.
6. In release mode, deserializeKeyInfo throws an error. indexOf doesn't really work on a function 👀
7. This error is hidden for the user. And the storage.writeData that actually triggers the persistance will never run again. This is because the defer boolean is never set to false again.
8. Changing the function a to function aWhateverElse makes it work.
9. Is this some bug with the rollup configuration used to build the library?

Sorry for the rambling above. Hopefully, it's actionable.

Expected behavior
The offline cache should work

Actual behavior
Nothing will be written to the cache. The cache will always be "{}".

[edenstrom]

Debugged a bit more by cloning the repo and trying some changes in the configuration.

Removing the plugin babel-plugin-closure-elimination changes function a(a) to just function(b).

[kitten]

I'm unmarking this as a bug for now because we've seen this before 😅 and it's a bug in React Native's build process. I believe they're using such an old version of uglify that this bug keeps coming back. We've now worked around it several times by changing our build output but we keep hitting different issues with it, so I'm kind of at the end of my wisdom with it 😅

I suppose we could switch off Closure Compiler and see how it goes but that's a big ask for fixing React Native but potentially increasing the size for web

Edit: to be fair, I think Closure Compiler never shipped that setting that'd turn off mangling, so maybe it is in fact time to drop it

[edenstrom]

Thanks for the quick response. Really annoying stuff. 😅

Good pointer. As a quickfix I'll use patch-package, but I'll try to modify the minifier config in metro.config.js and see if anything changes.

[edenstrom]

Btw @kitten, it's not closure compiler but this babel plugin: https://github.com/codemix/babel-plugin-closure-elimination

Not sure on the impact with/without it. 😅

I'll write back if I find something more. Thanks for everything. 😊

EDIT: https://github.com/FormidableLabs/urql/blob/main/scripts/rollup/plugins.js
(line 78)

EDIT 2: This PR could be relevant facebook/metro#643

[kitten]

Well, it's not 😅 it should be working just fine, but what we've seen before is that the old version of uglify that RN uses by default is really confused by this shadowed name

[kitten]

Oh, nice find! That PR is super relevant. I wasn't aware that this is in fact an issue in Metro itself! We'll have a small discussion internally on what we'll do about Closure Compiler in general. If we do decide to get rid of it in our build processes then it'll also get rid of mangled names, which in turn will work around this issue.