From 107f471c552a208384f276ce0d40b3d5f9cbb0e9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Sun, 16 Feb 2020 14:26:43 -0500 Subject: [PATCH 1/2] Remove fixPolyfills.ts, except when bundling for React Native. React Native used to have Map and Set polyfills that relied on tagging objects with a special __MAP_POLYFILL_INTERNAL_HASH__ property, which breaks down when the object to be tagged happens to be frozen. In 833072ee11a3c06f7d83964faae02e157f272dfb (#3964) I introduced a workaround to make sure objects got tagged before becoming non-extensible, which robustly solved the problem for any Map and Set polyfills that rely on an object tagging strategy. Note that Apollo Client freezes objects only in development (using maybeDeepFreeze), so the problem fixPolyfills.ts was intended to solve was only ever a problem in development. I also submitted a PR to React Native that make their Map and Set polyfills store non-extensible objects in an array, rather than attempting to tag them: https://github.com/facebook/react-native/pull/21492. Those changes were first released in React Native 0.59.0, so technically the fixPolyfills.ts logic should not be necessary for anyone using that version or later. Since then, React Native has stopped using any polyfills for Map and Set (yay!), so the need for the workaround has been even further reduced: https://github.com/facebook/react-native/commit/93b9ac74e59bbe84ea388d7c1879857b4acab114 Those changes were first released in React Native 0.61.0, which is still the latest minor version (0.61.5 is latest). I'm not sure how many people are still using older versions of React Native, or what sort of LTS policies they have. Expo SDK 36 uses 0.61.4, for what it's worth: https://docs.expo.io/versions/latest/sdk/overview/ In any case, I think we can eliminate these polyfills from the default bundle, as long as we take some care to include them when bundling for React Native. This strategy uses a combination of techniques for selective bundling in React Native: https://facebook.github.io/react-native/docs/platform-specific-code#platform-specific-extensions https://github.com/facebook/react-native/pull/2208 --- package.json | 5 +- src/cache/inmemory/fixPolyfills.native.ts | 53 ++++++++++++++++++++ src/cache/inmemory/fixPolyfills.ts | 60 +++-------------------- 3 files changed, 64 insertions(+), 54 deletions(-) create mode 100644 src/cache/inmemory/fixPolyfills.native.ts diff --git a/package.json b/package.json index 7b25e1da310..988d6c4a695 100644 --- a/package.json +++ b/package.json @@ -17,8 +17,11 @@ "module": "./dist/index.js", "types": "./dist/index.d.ts", "sideEffects": [ - "./dist/cache/inmemory/fixPolyfills.js" + "./dist/cache/inmemory/fixPolyfills.native.js" ], + "react-native": { + "./dist/cache/inmemory/fixPolyfills.js": "./dist/cache/inmemory/fixPolyfills.native.js" + }, "repository": { "type": "git", "url": "git+https://github.com/apollographql/apollo-client.git" diff --git a/src/cache/inmemory/fixPolyfills.native.ts b/src/cache/inmemory/fixPolyfills.native.ts new file mode 100644 index 00000000000..98b55ae5419 --- /dev/null +++ b/src/cache/inmemory/fixPolyfills.native.ts @@ -0,0 +1,53 @@ +// Make sure Map.prototype.set returns the Map instance, per spec. +// https://github.com/apollographql/apollo-client/issues/4024 +const testMap = new Map(); +if (testMap.set(1, 2) !== testMap) { + const { set } = testMap; + Map.prototype.set = function (...args) { + set.apply(this, args); + return this; + }; +} + +// Make sure Set.prototype.add returns the Set instance, per spec. +const testSet = new Set(); +if (testSet.add(3) !== testSet) { + const { add } = testSet; + Set.prototype.add = function (...args) { + add.apply(this, args); + return this; + }; +} + +const frozen = {}; +if (typeof Object.freeze === 'function') { + Object.freeze(frozen); +} + +try { + // If non-extensible objects can't be stored as keys in a Map, make sure we + // do not freeze/seal/etc. an object without first attempting to put it in a + // Map. For example, this gives the React Native Map polyfill a chance to tag + // objects before they become non-extensible: + // https://github.com/facebook/react-native/blob/98a6f19d7c/Libraries/vendor/core/Map.js#L44-L50 + // https://github.com/apollographql/react-apollo/issues/2442#issuecomment-426489517 + testMap.set(frozen, frozen).delete(frozen); +} catch { + const wrap = (method: (obj: T) => T): typeof method => { + return method && (obj => { + try { + // If .set succeeds, also call .delete to avoid leaking memory. + testMap.set(obj, obj).delete(obj); + } finally { + // If .set or .delete fails, the exception will be silently swallowed + // by this return-from-finally statement: + return method.call(Object, obj); + } + }); + }; + Object.freeze = wrap(Object.freeze); + Object.seal = wrap(Object.seal); + Object.preventExtensions = wrap(Object.preventExtensions); +} + +export {} diff --git a/src/cache/inmemory/fixPolyfills.ts b/src/cache/inmemory/fixPolyfills.ts index 98b55ae5419..4fbe4fd9965 100644 --- a/src/cache/inmemory/fixPolyfills.ts +++ b/src/cache/inmemory/fixPolyfills.ts @@ -1,53 +1,7 @@ -// Make sure Map.prototype.set returns the Map instance, per spec. -// https://github.com/apollographql/apollo-client/issues/4024 -const testMap = new Map(); -if (testMap.set(1, 2) !== testMap) { - const { set } = testMap; - Map.prototype.set = function (...args) { - set.apply(this, args); - return this; - }; -} - -// Make sure Set.prototype.add returns the Set instance, per spec. -const testSet = new Set(); -if (testSet.add(3) !== testSet) { - const { add } = testSet; - Set.prototype.add = function (...args) { - add.apply(this, args); - return this; - }; -} - -const frozen = {}; -if (typeof Object.freeze === 'function') { - Object.freeze(frozen); -} - -try { - // If non-extensible objects can't be stored as keys in a Map, make sure we - // do not freeze/seal/etc. an object without first attempting to put it in a - // Map. For example, this gives the React Native Map polyfill a chance to tag - // objects before they become non-extensible: - // https://github.com/facebook/react-native/blob/98a6f19d7c/Libraries/vendor/core/Map.js#L44-L50 - // https://github.com/apollographql/react-apollo/issues/2442#issuecomment-426489517 - testMap.set(frozen, frozen).delete(frozen); -} catch { - const wrap = (method: (obj: T) => T): typeof method => { - return method && (obj => { - try { - // If .set succeeds, also call .delete to avoid leaking memory. - testMap.set(obj, obj).delete(obj); - } finally { - // If .set or .delete fails, the exception will be silently swallowed - // by this return-from-finally statement: - return method.call(Object, obj); - } - }); - }; - Object.freeze = wrap(Object.freeze); - Object.seal = wrap(Object.seal); - Object.preventExtensions = wrap(Object.preventExtensions); -} - -export {} +// Most JavaScript environments do not need the workarounds implemented in +// fixPolyfills.native.ts, so importing fixPolyfills.ts merely imports +// this empty module, adding nothing to bundle sizes or execution times. +// When bundling for React Native, we substitute fixPolyfills.native.js +// for fixPolyfills.js (see the "react-native" section of package.json), +// to work around problems with Map and Set polyfills in older versions of +// React Native (which should have been fixed in react-native@0.59.0). From 07d052b72a46c064203bc576717b735c42692563 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 18 Feb 2020 12:23:54 -0500 Subject: [PATCH 2/2] Reduce bundlesize limit back down to 24KB. :tada: --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 988d6c4a695..c7580abce8a 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ { "name": "apollo-client", "path": "./dist/apollo-client.cjs.min.js", - "maxSize": "24.2 kB" + "maxSize": "24 kB" } ], "peerDependencies": {