sgarciac / bombadil Goto Github PK

I noticed the function fromCodePoint appears to expect a number. But it is called as follows:

  return fromCodePoint([parseInt(codeString, 16)]);

I don't see how to reconcile these except to remove the []s from around the result returned by parseInt.

I located this because I turned noImplicitAny on in the code and make sure that nothing ended up with implicit types and this popped out as something where I couldn't find a consistent type for the function argument.

PS - with the suggested change (and several others), I was able to create a version of bombadil that doesn't include any implicit types and still passes 100% of tests from #2. Shall I submit a PR with those changes and this fix?

This library looks really interesting and I need something like this. But potential issue I see is with the use of instanceof in discriminating your union types. I've used a similar approach in libraries before and been bitten badly. The issue arises when you instantiate instances of these classes from different packages/modules. They will not end up with the same class so instanceof will fail in surprising and unintuitive ways.

My solution has been to rework the code to use discriminated unions with explicit tags. I see no downside to this approach. Is this something you'd accept a pull request for?

if I create a simple toml with a repeating object declaration, I get an error in reader.errors.

[obj]
subobj.id = 123
subobj.id = 456

TypeError: Cannot read property 'type' of null.

It would be nice to have a nicer error in this case.

Thanks! Great lib!

toString = 1

This will make the parsing fail, and return null.

There is two way to fix this,

1. use hasOwnProperty, not in
2. use Object.create(null) to create a table, not {}

But the first way is slow, and can't deal with __proto__ property, so the way 2 is better.

Hello @sgarciac

I am working on a set of TOML tools here:

• https://github.com/bd82/toml-tools

Currently this includes a TOML Lexer/Parser using the latest Chevrotain Version.

• No transformer to JSON yet.

Perhaps you want to join efforts? 😄

Cheers.
Shahar.

Would you accept a PR that included some tests?

I've looked around in the code base and the tests but I can't find anywhere where can output to toml from either a json or a general object.

Is it it possible?

leave "moment" for luxon: https://moment.github.io/luxon

https://moment.github.io/luxon/#/why

reader.readToml('key = 0').result = null
reader.readToml('key = 0', true).result = { key: { type: 'atomicInteger', image: '0', value: 0 } }

Not sure if this is intended behaviour, but it's inconsistent with the TOML specification.
By the way, the errors object is empty, even when getting a null result

The same thing does not happen with integer value: 1
reader.readToml('key = 1').result = { key: 1 }