Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change expression parsers #1506

Closed
JustinGeorgi opened this issue Oct 5, 2022 · 4 comments · Fixed by #1522
Closed

Change expression parsers #1506

JustinGeorgi opened this issue Oct 5, 2022 · 4 comments · Fixed by #1522
Labels
enhancement New feature or request main ui Main UI

Comments

@JustinGeorgi
Copy link
Contributor

As the community gets more involved in advanced custom widget creation, more and more users are running up against the limitations of the expression-eval library. In particular, I've seen many issues with users wanting regex expressions or js methods that require functions as parameters. Expression-eval does not handle either of these two cases.

There's a recent fork of expression-eval, jse-eval, which adopts the jsep plug-in system which, in turn would allow the addition of regex and arrow plug-ins. Is it feasible/acceptable to move MainUI from expression-eval to jse-eval?

@JustinGeorgi JustinGeorgi added enhancement New feature or request main ui Main UI labels Oct 5, 2022
@ghys
Copy link
Member

ghys commented Oct 5, 2022

Yes I knew about jse-eval and thought the same thing, but didn't look into whether or not it could be a drop-in replacement for expression-eval (or only minor changes). The goal would be to allow the extra features without breaking any existing expressions, of course.

I think it could work - it's certainly worth looking at.

@JustinGeorgi
Copy link
Contributor Author

I was thinking of putting together a PR with a few other little tweaks to the widgets anyway. Since you're OK with it, I'll take a careful look at jse-eval and see if it's compatible while I'm doing that.

@ghys
Copy link
Member

ghys commented Oct 6, 2022

I think it might be fair to ping @6utt3rfly so that they know their work is being considered for inclusion as a replacement of expression-eval, as they might have additional insight.

There's an established ecosystem around these expressions, so it would be unwise to make any breaking change without careful consideration.

@6utt3rfly
Copy link

Thanks for the ping @ghys 🙂 . I had originally created the jse-eval fork, and PR to update expression-eval, but after waiting awhile, ended up publishing it as a separate package. I'm happy to help or update things if there are any conflicts between the two packages, but I believe jse-eval should be an easy drop-in replacement. It uses a newer (major) version of jsep though, which may cause some breaking changes for certain use cases (see v1.0.0 release notes). Even though jse-eval doesn't mention an eval export like expression-eval's docs, jse-eval kept it for compatibility and added evaluate to avoid warnings from lint and other tools about using eval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants