Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setAsEmptyObject - null Exception #107

Closed
gonenduk-dy opened this issue May 9, 2023 · 6 comments
Closed

setAsEmptyObject - null Exception #107

gonenduk-dy opened this issue May 9, 2023 · 6 comments

Comments

@gonenduk-dy
Copy link

gonenduk-dy commented May 9, 2023

Hi @kriszyp ,

I am running the test code attached below on Node.js 18.16 with msgpackr 1.9 using the 2 new flags and get this error:

TypeError: Cannot set properties of null (setting '10')
    at Packr.writeObject.useRecords.variableMapSize.target.<computed> (/Users/.../node_modules/msgpackr/dist/node.cjs:1683:24)

When turning off the 2 flags, it works well.

Thanks,
Gonen.

The test code:

const { Packr } = require('msgpackr');
const msgpackr = new Packr({ useRecords: false, encodeUndefinedAsNil: true, variableMapSize: true, mapAsEmptyObject: true, setAsEmptyObject: true  });

const map = new Map();
map.set('a', 1);
map.set('b', 2);
const set = new Set();
set.add('a');
set.add('b');
const input = { map, set };

const packed = msgpackr.pack(input);
const unpacked = msgpackr.unpack(packed);
console.log(JSON.stringify(unpacked));
@gonenduk-dy gonenduk-dy changed the title mapAsEmptyObject setAsEmptyObject - Cannot set properties of null mapAsEmptyObject setAsEmptyObject - null Exception May 10, 2023
@gonenduk-dy
Copy link
Author

@kriszyp
Any update?

@kriszyp
Copy link
Owner

kriszyp commented May 15, 2023

Yes, and I don't know why github isn't properly showing it, looks linked to me, but added a test for this here ab6dcc4, and it doesn't seem to reproduce for me. I am doing something wrong?

@gonenduk-dy gonenduk-dy changed the title mapAsEmptyObject setAsEmptyObject - null Exception setAsEmptyObject - null Exception May 17, 2023
@gonenduk-dy
Copy link
Author

hey @kriszyp,

I deleted my previous comment and write this one. It is much more simple.

I cloned your repo and wrote a test. Everything works, just like your unit test, as long as I import the lib locally (not using npm install).
When I import the lib in a different project after npm install - it fails.

Note: Doesn't matter if I use commonJS or ES module. Same results.

Any idea why this is happening?

The test code:

// Working from inside your repo:
import * as msgpackr from './node-index.js'

// Not working (from any other project after npm install msgpackr@1.9.0):
import * as msgpackr from 'msgpackr';

const { Packr } = msgpackr;
const packr = new Packr({ useRecords: false, encodeUndefinedAsNil: true, variableMapSize: true, mapAsEmptyObject: true, setAsEmptyObject: true });

const set = new Set();
set.add('a');
const input = { set };

const packed = packr.pack(input);
const unpacked = packr.unpack(packed);

@kriszyp
Copy link
Owner

kriszyp commented May 17, 2023

For some reason the built/dist version in the last release is not up-to-date with the source. I will release a new version, and make sure it is fully built, as soon as I figure out how to fix #109 .

@kriszyp
Copy link
Owner

kriszyp commented May 17, 2023

Published an update in 1.9.2.

@gonenduk-dy
Copy link
Author

gonenduk-dy commented May 17, 2023

Cool Thanks!

Verified the new version and it does solve the issue.
Closing the issue.

Thanks again!
Appreciate your huge effort and quick response.

P.S. Will continue to test backward compatibility. If we find anything - will let you know in yet another issue ;)

SPKorhonen pushed a commit to SPKorhonen/msgpackr that referenced this issue May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants