Closed raidancampbell closed 3 years ago
Looks promising!
update:
org
-> com
change. not sure if you carevar
to prevent scope leakage. setting use strict
should make this a visible error, but Otto doesn't support it. Goja does support it: I gave up on refactoring that, it's a larger change than I'm looking to make. Instead the client/user needs some way of ensuring the code is compliant.eval
. Replacing newlines with semicolons causes collateral damage for code that's formatted like var x = {\n foo: bar,\n baz: qux\n}
. At this point the solution is fraught with enough unexpected pitfalls that I'd be unlikely to use it myself, as this implementation would require a significant amount of rule rewrites. Unless you wish otherwise, I'll close this in a couple days.
Thanks for the update.
Re TravisCI, I opened #82 to track the required updates.
Re edge cases, pitfalls, and required rule updates: Understood. Sorry about that, but glad the other performance improvements resulted in a tolerable situation.
Thanks again for the analysis and related work.
Closing as discussed above: The suggested implementation had too many pitfalls to be trustworthy
Not worth an in-depth review yet, just looking to make sure I'm steering the implementation in a direction you'd want.
WIP until:
enable strict mode when the flag is set so that javascript will fail rather than leak context inx = 1
declarationsEDIT: otto doesn't understand strict mode. this will need a switch to goja.return eval('%s')
so thatreturn
isn't requiredThere's some unrelated test cleanups to make it run faster, 0a4244c is the big ticket commit here. happy to rip them out so that there's a single useful commit in this PR.