encoder: Encode instances of Number as numbers, not maps by FlorianDenis · Pull Request #271 · msgpack/msgpack-javascript · GitHub
Skip to content

encoder: Encode instances of Number as numbers, not maps#271

Open
FlorianDenis wants to merge 2 commits into
msgpack:mainfrom
FlorianDenis:main
Open

encoder: Encode instances of Number as numbers, not maps#271
FlorianDenis wants to merge 2 commits into
msgpack:mainfrom
FlorianDenis:main

Conversation

@FlorianDenis

@FlorianDenis FlorianDenis commented Mar 7, 2025

Copy link
Copy Markdown

Make sure instances of Number as encoded as a number instead of a map.
I am encountering this issue in my current project, I believe this is a more sensible default behavior

All tests pass.

@codecov-commenter

Copy link
Copy Markdown

@gfx

gfx commented Mar 8, 2025

Copy link
Copy Markdown
Member

Why do you use the Number wrapper object? There's no reason to use it and it seems just a bug, in your project.

@FlorianDenis

Copy link
Copy Markdown
Author

I am not going to go too much into the details of my project because this is not the place, but it turns out that I have a need for representing a integer using an object instance rather than a primitive type, a need which Number was built into the language to fulfill. This is a deliberate choice, not a bug.

My goal with this PR is merely to point out that Number does exist today as a core part of the language (has since ES1, really), and so the question this poses here is:

  • Do we consider the MessagePack representation of it should be a map
  • Or do we consider it should be a int/float?

@FlorianDenis

Copy link
Copy Markdown
Author

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

Successfully merging this pull request may close these issues.

3 participants