What should the convention be for constants? about bs-webapi-incubator HOT 10 CLOSED

Verbose is fine; consumers wouldn't notice the difference. They also don't create additional bindings and get inlined.

from bs-webapi-incubator.

_MyConstant is kind of an awkward middle between myConstant and _MY_CONSTANT. I prefer myConstant but maybe we got counterexamples.

from bs-webapi-incubator.

Some more context on different types of constants, how the JS API implements them and how they're currently modeled in reason-js:

• Enum values:
  • In JS: PROCESSING_INSTRUCTION_NODE
  • Current implementation in Dom: Variant with type constructors in the form of e.g. ProcessingInstruction
  • Current implementation in Gl: let binding in the form of e.g. _LINE_STRIP
• Bitmask flags:
  • In JS: SHOW_DOCUMENT_TYPE
  • Current implemtnation in Dom: module with let bindings in the form of e.g. WhatToShow._DocumentType
  • Current implementation in Gl: let binding in the form of e.g. _STENCIL_BUFER_BIT

from bs-webapi-incubator.

let bindings shouldn't be necessary there right? Just externals? I don't know why those are lets.

Since we do use variants too, we can probably be idiomatic here and use lineStrip, documentType and stencilBufferBit? Hopefully this doesn't clash with an actual lineStrip variable?. Otherwise it should probably be _LINE_STRIP, etc.

from bs-webapi-incubator.

externals would be more verbose, I guess?

It feels a bit inconsistent to me to have bitmask flags in camelCase while enums, being modeled as variants, would have to be PascalCase. But in either case they should probably both be wrapped in type-specific modules, which would make name collisions a non-problem.

from bs-webapi-incubator.

I think I'm leaning towards the following:

enums:

module NodeType = {
  type t =
  | Element
  | Attribute
  | ...

  let encode : t -> string = ...
  let decode : string -> t = ...
};

bitmask flags:

module WhatToShow = {
  type t;

  external all : t = "NodeFilter.SHOW_ALL" [@@bs.val];
...
  external processingInstruction : t = "NodeFilter.SHOW_PROCESSING_INSTRUCTION" [@@bs.val];
...
};

from bs-webapi-incubator.

humm that's a bit much. Not sure we should take the occasion to rearrange things like this. I still prefer either myConstant or _MY_CONSTANT

Btw I've also used quite a few [@@bs.val] this way. @bobzhang are things like NodeFilter.SHOW_ALL" [@@bs.val] ok?

from bs-webapi-incubator.

I'm not sure I understand. Converting the variants to externals would be a much bigger rearrangement, is non-idiomatic and misses out on exhaustiveness-checking. And not namespacing with a module will require namespacing in the names themselves (The variants are already overlapping, but they can be, because they're variants).

The bitmask flags is already a module, in part because it provides a function to safely compose the flags. This should probably be functionality provided by some standard library or other, but there's of course no consensus on that either yet (I have my own implementation in rebase though).

from bs-webapi-incubator.

it depends on how those enum variant will be used, will you do OPEN | WRITE or not?

from bs-webapi-incubator.

For the WhatToShow bitmask flags type I have a (poorly named) many function that will lor flags together in a type safe manner. I'd like to keep the type abstract.

from bs-webapi-incubator.