lark-parser / Lark.js

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

Pain points with the examples and API, plus recommendations #1

Open jgonggrijp opened 3 years ago

jgonggrijp commented 3 years ago

First, some positive notes.

General pain points and recommendations:

Pain points and recommendations specific to the example JSON parser:

Pain points and recommendations specific to the example Python 3 parser:

node run_python_parser.js
__main__.py 2901
Lark.js/examples/python_parser.js:2912
        throw e;
        ^

SyntaxError: Invalid regular expression: /(?:(?i:\d+j)|(?i:((\d+\.[\d_]*|\.[\d_]+)(e[-+]?\d+)?|\d+(e[-+]?\d+)))(?i:j))/: Invalid group
    at new RegExp (<anonymous>)
    at Object.compile (Lark.js/examples/python_parser.js:41:12)
    at _get_match (Lark.js/examples/python_parser.js:47:17)
    at _create_unless (Lark.js/examples/python_parser.js:1832:17)
    at TraditionalLexer._build_scanner (Lark.js/examples/python_parser.js:1984:34)
    at TraditionalLexer.get scanner [as scanner] (Lark.js/examples/python_parser.js:2014:12)
    at TraditionalLexer.match (Lark.js/examples/python_parser.js:2021:17)
    at TraditionalLexer.next_token (Lark.js/examples/python_parser.js:2042:18)
    at ContextualLexer.lex (Lark.js/examples/python_parser.js:2168:21)
    at lex.next (<anonymous>)
erezsh commented 3 years ago

Thanks!

I agree with all your comments.

For these reasons, I strongly recommend generating the output as ESM instead. You can optionally generate UMD and pure CommonJS formats as well, by doing a secondary Rollup pass over the ESM version as an added service to the user.

Please expand on this point. What changes do I need to make?

jgonggrijp commented 3 years ago

Instead of

function load_parser(option = {}) {/*...*/}

//...

module.exports = {
    //...,
    load_parser,
};

you do

export function load_parser(options = {}) {/*...*/}

The optional secondary Rollup pass is a bit involved, as it requires installing npm modules. This could be deferred to the user.

erezsh commented 3 years ago

I see. Is there a short-hand to export all the symbols at once?

jgonggrijp commented 3 years ago

Yes, you can also do


export {
    //...
    load_parser,
};
thekevinscott commented 6 months ago

I think these are some great comments. I would in particular like to second the request for ESM support, which I think is as simple as updating the export syntax to ESM (that said, I've no idea if other Lark.js consumers expect CJS output, so I don't know how disruptive this change would be).

I don't think it's necessary to provide different flavors of the output file (CJS / UMD) as most of the ecosystem supports ESM natively, and for those that don't transforming is commonly an end-user requirement.

Two other pain points I'd throw out there: