exprs is not exported even though it is use by @ohm/cli

[ericmorand]

Hi guys,

I'd like to be able to generate code from a grammar, like @ohm/cli is doing. But it seems like the package doesn't export the node classes themselves (like Apply or UnicodeChar for example) even though @ohm/cli is perfectly able to use them, as can be seen here:

https://github.com/harc/ohm/blob/211aed7660064d1f64e063c7481283882a564f68/packages/cli/src/helpers/getNodeTypes.js#L38

What is this exprs property that is supposedly exported but is not, at least based on the TypeScript declaration?

And if it is not public, what is the recommended way to identify a Node? Should we officially rely on constructor.name? Doesn't it make parsing more difficult than needed by obfuscating artificially nodes typology?

[pdubroy]

I don't see any reason why we can't add this to type definitions. I don't think it was intentionally omitted, it's just not really part of the core API of Ohm, so it was never included.

I'll see if I can fix this sometime this week.

[pdubroy]

This has been fixed and release in v16.4.0.

Please let me know if you run into any issues, or if there's anything else that you think should be added to the typings.

[ericmorand]

Thanks a lot.

Although, I'm afraid the change did create a breaking change:

This was the previous type definition of PExpr:

interface PExpr {}

This is the new type definition of PExpr:

class PExpr {
    getArity(): number;
    isNullable(): boolean;
    toString(): string;
    toDisplayString(): string;
  }

Grammars generated with pre-16.4.0 CLI can't be used as namespace to create grammar with 16.4.0.

The reason is that grammar(source, namespace) expect namespace to be an object where values are of type Grammar. But Grammar 16.4.0 type is not the same as Grammar 16.3.4 type, since PExpr is different in 16.4.0 and not backward compatible.

Hence, as happy as I am with the change, I recommend that you revert it and wait for the next major release, to preserve others from facing a regression just to please one person - myself. :)

[pdubroy]

Thanks for the feedback. Re-opening this until the issue is resolved.

I'm not sure I understand how grammars generated with pre-16.4.0 are incompatible with this. I just quickly tried using 16.4.0 to compile against a bundle that was generated with 16.3.4, and didn't have any issues.

Bundles aren't really tied to the version they are generated with, they should their type definitions from the version they are loaded with. Here's the top of a generated .d.ts:

import {
  ActionDict,
  Grammar,
  IterationNode,
  Node,
  NonterminalNode,
  Semantics,
  TerminalNode
} from 'ohm-js';

If we renamed or removed classes here, I can see how it would be a problem, but it's not clear to me why the changes I made to PExpr would cause any problems here. Can you give some more details about the problems you are seeing?

[pdubroy]

As far as I know, there isn't an actual bug here, so closing for housekeeping purposes. Please feel free to reopen if I'm wrong.