hapipal / confidence

Dynamic, declarative configurations
Other
263 stars 43 forks source link

Random ordering occurs when there is a dash '-' in element key names #40

Closed biant closed 9 years ago

biant commented 9 years ago

We use confidence for processing package.json files, these usually have packages with dashes in the names, e.g. good-broadcast
When these files are processed by confidence, the output has element keys reordered in a seemingly random fashion. just try the following input data:

{
    "A-1": "I should be first", 
    "B": "runner up", 
    "C": 50 
}

This randomization can occur at any level of nesting within the input structure so long as there is a dash in a key name.

This is severely hampering our ability to determine clear differences between subsequent applications of confidence. What's worse is if there is no conditional processing required, the output is still jumbled.

patrickkettner commented 9 years ago

Hi @biant! Always a pleasure to hear from you

Sooooo, technically your keys are invalid. From the readme

key names can only contain alphanumeric characters and '_' with the '$' prefix reserved for special directives.

Do you need -s or are you just happening to use them?

biant commented 9 years ago

Hi Patrick! the dashes are beyond my control as they are NPM package names in a package.json file. We use confidence to select different versions/tags of packages for different environments. This also occurs in HAPI plugin names in a HAPI configuration file. The randomization is totally perplexing to me, if dashes were not supported, I would expect confidence to error out or omit the bad key names in the output (not that I would want this behavior)

Marsup commented 9 years ago

Javascript doesn't guarantee the order of the keys, that's in the specs, you shouldn't rely on that.

devinivy commented 9 years ago

That's true. This may help with that, @biant: https://www.npmjs.com/package/sorted-object. I think there's more general question here, though: are hyphens needed for common use-cases of confidence?

kpdecker commented 9 years ago

A few comments:

  1. There should be no restriction on key names unless there are technical ones. The $ prefix being reserved being the primary one that I'm aware of. Unicode folks need love too.
  2. Key ordering on enum is a touchy issue prior to ES6. It's not speced but the defacto behavior of engines is that creation order wins in most APIs. Every time you save JSON to a repository you rely on this (see package.json when not manually edited via --save, etc).

This specific issue is something to do with getOwnPropertyNames in Hoek.clone. Under 0.10.x the key order is random when the - comes into play. Under 0.12.x and I presume the iojs series, the order of getOwnPropertyNames matches that of keys and seemingly the ES6 spec.

So this seems like an V8 engine bug to me the is highly unlikely to be fixed under older Node instances but I wasn't able to track down exact fix versions or anything like that.

var Confidence = require('./');
var store = new Confidence.Store();

var document = {
    a: 1,
    b: 2,
    'foo-1': 4,
    c: {
        $filter: 'size',
        big: 100,
        small: 1,
        $default: 50
    }
};

console.log(Object.getOwnPropertyNames(document), Object.keys(document));
store.load(document);
console.log(store._tree);
biant commented 9 years ago

But alce does maintain the order of keys. I fully understand the implications of the JSON spec, but that does not help my particular usage.

biant commented 9 years ago

Hi Kevin, I'll give node 12.x a try.

biant commented 9 years ago

works for me! closing this issue