Behavior on undefined

[Jack-Works]

I'm switching from another lib to typeson and I find my app is broken.

t = require('typeson/dist/typeson') 
q = new t().register([require('typeson-registry/dist/presets/builtin')])
o = q.parse(q.stringify({a: undefined}))
// return { a: Undefined {} }, should be undefined instead of Typeson.Undefined

o.a.constructor.toString()
// 'function Undefined(){_classCallCheck(this,Undefined)}'

[brettz9]

Strange... Looks like a build problem as I can replicate this, but the problem does not occur if using the main export:

// Using path from within the project
import Typeson from './dist/index.js';

const {builtin} = Typeson.presets;
const typeson = new Typeson().register([builtin]);

For now, I'd get the builtin preset off of the main export.

I hope to look at this in the next day or so.

[Jack-Works]

hope for the fix, thanks!

[brettz9]

Actually, it looks like its an issue between the typeson copy and the typeson-registry copy. Still need to get a fix, but for now, you should be able to get what you want without losing anything by just using:

t = require('typeson-registry') 
q = new t().register([require('typeson-registry/dist/presets/builtin')])
o = q.parse(q.stringify({a: undefined}))

[Jack-Works]

I'm sorry, but it's not working for me

[brettz9]

Are you testing with npm releases or master or ?

[brettz9]

And if npm, installed via yarn or npm?

[Jack-Works]

I'm using latest version installed by yarn

typeson-registry@^1.0.0-alpha.29: version "1.0.0-alpha.29" resolved "https://registry.yarnpkg.com/typeson-registry/-/typeson-registry-1.0.0-alpha.29.tgz#df53c20fc02ec96ffc991161c3dd2c121b037d24" integrity sha512-DqRoIx0CtmBGXuumFk7ex5QE6JCWHNKry6D1psGUUB9uIPRrj/SCtuRAidZjLgieWpwn1EfrTFtG0IN2t//F8A== dependencies: base64-arraybuffer-es6 "0.5.0" typeson "5.13.0" whatwg-url "7.0.0"

typeson@5.13.0, typeson@^5.13.0: version "5.13.0" resolved "https://registry.yarnpkg.com/typeson/-/typeson-5.13.0.tgz#dc65b23ea1978a315ed4c95e58a5b6936bcc3591" integrity sha512-xcSaSt+hY/VcRYcqZuVkJwMjDXXJb4CZd51qDocpYw8waA314ygyOPlKhsGsw4qKuJ0tfLLUrxccrm+xvyS0AQ==

[brettz9]

Ok, I think I see the issue, thanks, will try to fix.

[brettz9]

I'm afraid it should take some more time.

I've come down with a cold now, and it is not an easy fix. In order to determine constructors (as I recall for the sake of cross-environment, cross-browser support (and I'm pretty sure I would have checked to see if Babel and any Symbol.hasInstance polyfills it might have would work for us), we are getting cross-frame-friendly instance checks by stringifying and comparing constructor functions, but unfortunately the versions end up different because some variables that Babel adds are incremented differently in our different versions.

Add on top of that, our aiming for compatibility between minified and non-minified versions of Typeson/Typeson-registry, with Rollup auto-inserting semi-colons (and the Babel plugin somehow avoiding this), making our internal use of this instance checking against the Undefined and TypesonPromise constructors, there is unfortunately more to it which will need some energy to tackle. It's not anything completely mystifying, but expect some time.

My health is not good to begin with, but hopefully, I'll have some energy over the next few days, but no guarantees.

[Jack-Works]

Wish you have a speedy recovery!

If I can make sure my code will run in ES2017+ compatible environment, can I skip the babel part and the polyfill part?

[brettz9]

The Babel part, I would guess so, though if for Node, you will presumably need polyfills for Blob/File. See the test environment for what you may need to set up (if not the source files alone); even the great jsdom does not cover everything.

And if you target the source files, you will need to bundle, as we do use some third party deps. Take a look at build.js for how we roll it up.

[Jack-Works]

Hi, any progress on this and, do you need any help?

[brettz9]

Hi, I'm afraid my concentration is limited to mundane tasks still, and this is fairly involved. Thanks for the offer of help, but unless you wanted to really dig into the dirt, it doesn't look like something easy to resolve. This is still a top priority for me as soon as I have some energy though.

[brettz9]

Sorry, haven't forgotten--cold is just dragging on.

[Jack-Works]

I'm here to help these days if there is no progress on this. Our project is blocking on this 🙈

[brettz9]

I'm finally starting to feel a bit better, so I should at least be able to help point you in the right direction. I'll try to get some info out to you shortly.

[Jack-Works]

https://github.com/dfahlander/typeson/blob/c2d7277cbf052b592d9f884795bd671c4961b20a/typeson.js#L870

                    if (hasConstructorOf(val, Undefined)) {
                        clone[k] = undefined;
                    } else if (val !== undefined) {
                        clone[k] = val;
                    }

When I reach this line, the constructed Undefined object is not an instance of the typeson Undefined object so the Undefined object is kept

[Jack-Works]

Oh I see the comment

/**
 * We keep this function minimized so if using two instances of this
 * library, where one is minimized and one is not, it will still work
 * with `hasConstructorOf`.
 * @constructor
 */

maybe that is not the reason

[brettz9]

Yes, that is the source of the issue, and the fact that we need to get Rollup to either consistently stop adding semi-colons, or, as with the Babel plugin, consistently get it to strip semi-colons.

[Jack-Works]

Ok I got the reason

/**
 * This function is dependent on both constructors
 *   being identical so any minimization is expected of both.
 * @param {Any} a
 * @param {function} b
 * @returns {boolean}
 */
function hasConstructorOf (a, b) {
    if (!a || typeof a !== 'object') {
        return false;
    }
    const proto = getProto(a);
    if (!proto) {
        return false;
    }
    const Ctor = hasOwn.call(proto, 'constructor') && proto.constructor;
    if (typeof Ctor !== 'function') {
        return b === null;
    }
    return typeof Ctor === 'function' && b !== null &&
        fnToString.call(Ctor) === fnToString.call(b);
}

Here use toString to check if they share a same instance

e is the Undefined instance, t is the Undefined constructor. And I think it is not the problem of semicolons....

[brettz9]

The semi-colons affect how the stringification occurs, at least between the minified and non-minified versions, an issue that has to be fixed regardless. ~But IIRC, that may also be the issue with the typeson-registry dist versions vs. Typeson.~ Sorry, just noticed your screenshot, yeah that numbering was actually it.

[Jack-Works]

It may be simpler to use a string on the constructor like __typeson__class__undefined__ to identify the class

[brettz9]

That sounds like it might work; we just need the PR to add for TypesonPromise as well, as that no doubt suffers from the same issue.

[brettz9]

As long as we don't break from past stringification as that would be a breaking change.

[Jack-Works]

And maybe an ES6 build will resolve the problem too. Not all environment need class transpilation and Symbol polyfill etc

[brettz9]

We can add such a build, but:

1. I'd like to fix this so there is interoperability among all builds
2. We can't practically add ES6 builds in addition to the Babel ones for all individual types/presets; but if like the index.js build sure. (FWIW, the ESM build already foregoes Babel.)

[Jack-Works]

Hi Do you need a PR from me?

[brettz9]

I'm sorry, I thought I'd have the energy after some rest, but still isn't enough. I may get a second wind later, but your chances are better to get one faster if you could prepare it, yes.