search

lark-parser / Lark.js

Live port of Lark's standalone parser to Javascript
MIT License
71 stars 12 forks source link

Missing "strict" option #37

Closed micaww closed 1 month ago

micaww commented 11 months ago

The strict option in Lark 1.1.7 is not in the list in lark-js, causing runtime errors with the standalone parser if it was generated using Lark 1.1.7.

Side question: Why is a runtime error thrown in the first place for unknown options? This should either be a compile-time error or unknown options ignored. I shouldn't have my CI pass but now unnecessarily throwing at runtime because "strict" option shouldn't be there but is being generated by Lark.

erezsh commented 11 months ago

Why is a runtime error thrown in the first place for unknown options?

Some future options may introduce meaningful changes to how the parser works, which would cause a much more cryptic error. But it's true that in case of strict, it's safe to ignore it.

For lark-js flows, it's probably a good idea to keep your lark version fixed to a specific version, and upgrade manually when you need a new feature or fix. Lark is already fairly mature, so there isn't much to improve in generating the grammar.

Anyway, thank you for bringing it to my attention. I will try to fix it soon.

erezsh commented 11 months ago

P.S. I also agree that if it can be made into a compile-time error, that's better. I will look into it.

micaww commented 11 months ago

Thanks for the quick response!

That suggestion is helpful, thank you; is it documented anywhere which version of Lark that Lark-js supports / is transpiled from? If the version numbers were matched up it would be easy for us to just pin them to the same version, but it doesn't seem obvious from looking at Lark-js.

micaww commented 11 months ago

Also; worth noting that since lark-js depends on lark, if someone just does pip install lark-js right now without first pinning lark, lark-js will generate invalid output because it will install the latest version of lark (if i understand how pip works correctly). And it's not obvious from the readme that lark-js actually depends on running lark in order to generate, which may steer new users away from trying it. Seems we need to first pin lark@1.1.6 before even installing lark-js to avoid errors.

Our migration to Lark is 99% driven from the JS/python interop so that we can have parsing of the same grammar on frontend & backend, so lark-js is really important for our use case. What you've made is very cool and has opened up a lot of possibilities for our use case, great work!

Thanks for your help!

thekevinscott commented 7 months ago

Just ran into this error (I think it's the same error?). Did a pip install lark-js --upgrade and tried to run through the example - got:

      throw new ConfigurationError(format("Unknown options: %s", dict_keys(o)));
      ^

ConfigurationError: Unknown options: strict,ordered_sets

For me, 1.1.6 still reported an error about strict, but downgrading lark to 1.1.5 seems to work.

I second @micaww's comment above, it would be nice to drop a note in the readme indicating the need to pin lark (if that is indeed the right approach, I'm very new to this project).

erezsh commented 6 months ago

The ideal approach would be to update Lark.js to support the latest Lark. However, I'm a bit busy these days, so I don't know when I'll be able to get to it.

So, pinning the Lark version is probably a good idea! I will try to get to it this week.

erezsh commented 6 months ago

Added a PR: https://github.com/lark-parser/Lark.js/pull/41