Skip to content

Commit

Permalink
Remove fixPolyfills.ts, except when bundling for React Native.
Browse files Browse the repository at this point in the history
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 833072e (#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: facebook/react-native#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:
facebook/react-native@93b9ac7

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
facebook/react-native#2208
  • Loading branch information
benjamn committed Feb 18, 2020
1 parent da8c3c3 commit 5940397
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 54 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"module": "./dist/index.js",
"types": "./dist/index.d.ts",
"sideEffects": [
"./dist/cache/inmemory/fixPolyfills.js"
"./dist/cache/inmemory/fixPolyfills.native.js"
],
"repository": {
"type": "git",
Expand Down
53 changes: 53 additions & 0 deletions src/cache/inmemory/fixPolyfills.native.ts
Original file line number Diff line number Diff line change
@@ -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: <T>(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 {}
60 changes: 7 additions & 53 deletions src/cache/inmemory/fixPolyfills.ts
Original file line number Diff line number Diff line change
@@ -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: <T>(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 [email protected]).

0 comments on commit 5940397

Please sign in to comment.