mafintosh / mafintosh/protocol-buffers
Use closures instead of genfun?
- Dominant language
- JavaScript
- Stars
- 760
- Forks
- 75
- PR merge metrics
- No merged PRs in 30d
Description
Your compile logic currently uses [generate-function](https://github.com/mafintosh/generate-function) to build some code, e.g. https://github.com/mafintosh/protocol-buffers/blob/012e982de02db16693ac777816677cbf6f38697e/compile.js#L105.
This is neat (it looks like in some cases you are dynamically including/excluding entire sections of code!) — however it does mean that it won't work on the current [Tessel](https://tessel.io/) runtime, where JavaScript code is actually compiled "offline" before it is pushed, and later `eval`/`Function` is [not currently possible](https://github.com/tessel/runtime/issues/220#issuecomment-49558098).
I know this is just one platform, and it's not really your fault that its "JavaScript" implementation is incomplete, but this was kind of a bummer. Protocol buffers seem like a great fit there. (FWIW, you may also run into this restriction in-browser with default [Content Security Policy](https://developer.chrome.com/extensions/contentSecurityPolicy#JSEval) settings in affect.)
Anyway, just thought I'd mention this for future consideration. I know performance is a big focus here, but seems like simply returning closures instead of "generated functions" would bring compatibility (and readability) benefits, and [speculatively] might not be appreciably slower on modern optimizing JS engines.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in compile.js around line 105 and inspect how generate-function builds the protocol-buffer code. Compare the generated-function approach with the proposed closures, including the Tessel runtime and browser Content Security Policy constraints. Done means the compatibility and readability trade-offs are resolved and the compile path no longer depends on runtime eval or Function if that approach is accepted.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100