fix: IE11 clone() #1029
fix: IE11 clone() #1029
Conversation
Moving `_whitelist` and `_blacklist` initializations inside `proto` fixes: `createSetIterator called on incompatible receiver` error in IE11
|
Not sure why CI didn't run here, but this likely breaks a lot of things...I don't think this is fixing the issue correctly, but it's hard to understand what the IE error even is here. |
src/mixed.js
Outdated
| @@ -92,6 +89,9 @@ const proto = (SchemaType.prototype = { | |||
| __isYupSchema__: true, | |||
|
|
|||
| constructor: SchemaType, | |||
|
|
|||
| _whitelist: new RefSet(), | |||
| _blacklist: new RefSet(), | |||
There was a problem hiding this comment.
this moves the properties to the prototype which is incorrect. They are instance properties and should not be shared between all instances of SchemaType, which is what would happen here.
|
To be able to replicate the IE11 failure, first prepare the build for es5 commonjs: const nodeResolve = require('rollup-plugin-node-resolve');
const commonjs = require('rollup-plugin-commonjs');
const babel = require('rollup-plugin-babel');
const filesize = require('rollup-plugin-filesize');
const base = {
input: './src/index.js',
plugins: [
nodeResolve({
main: true,
browser: true
}),
commonjs(),
babel({
babelrc: false,
presets: [[
'@babel/preset-env', {
targets: {
ie: '11'
}
}
]],
}),
],
};
module.exports = [
{
...base,
output: [
{
file: 'dist/yup.js',
format: 'iife',
},
],
plugins: [...base.plugins, filesize()],
},
];Then, expose all yup functions to global window object from within src/index.js. <html>
<head>
<script src="some-default-es5-polyfills.js"></script>
<script src="dist/yup.js"></script>
</head>
<body>
<script>
const object = window.yup.object;
const string = window.yup.string;
const number = window.yup.number;
const date = window.yup.date;
function testRun() {
let s = string();
try {
s = s.required().concat(string()).concat(null);
} catch (ex) {
console.error(ex.stack);
}
const contactSchema = object().shape({
name: s
}).shape({
age: number()
.required()
.positive()
.integer(),
email: string()
.email(),
website: string()
.url(),
createdOn: date()
.default(function () { return new Date() })
}).shape(null);
contactSchema.isValid({
// name: 'jimmy',
age: 24,
email: 'jdog@cool.biz'
}).then(console.log);
contactSchema.isValid({
name: 'jimmy',
age: 24,
email: 'jdog@cool.biz'
}).then(console.log);
}
testRun();
</script>
</body>
</html>Running this in IE11 breaks on My PR fixes this bug. It doesn't break anything else as far as I know. |
|
Thanks for providing more context! It looks like this only fixes the issue because it avoids trying to clone the RefSets, but it creates a subtle and bad bug where one instance of these RefSets is shared between all schema globally. Judging from the error it looks like the actual bug is in the Concat method, but not sure where. And to be clear even with es6 classes this would still not work. The code as it's is functionally the same as an es6 class |
|
Thanks for responding! I ran my local tests badly, which misled me that everything was fine. |
|
Also, |
Improvement on previous commit
|
For some reason, the RefSets aren't cloned during the SchemaType.clone and this causes all of the IE11 bugs. This new commit doesn’t break any of the 536 tests, would you accept it? |
Moving
_whitelistand_blacklistinitializations insideprotofixes:createSetIterator called on incompatible receivererror in IE11