feat: introduce API parity between JS and WASM implementation #27
Conversation
| @@ -19,7 +19,9 @@ npm install cjs-module-lexer | |||
| For use in CommonJS: | |||
|
|
|||
| ```js | |||
| const parse = require('cjs-module-lexer'); | |||
| const { parse, init } = require('cjs-module-lexer'); | |||
There was a problem hiding this comment.
not sure if we should require it to be called or not. E.g. what happens if I use import('cjs-module-lexer')? Possibly confusing. Dunno 🙂
There was a problem hiding this comment.
since we're changing the API regardless it might be worth it. happy to make the change if you want 👍
There was a problem hiding this comment.
Strictly speaking, the way the exports are defined in https://github.com/guybedford/cjs-module-lexer/blob/master/package.json#L6, import() should always get the ES module form by definition of the resolution rules and even in bundlers (which RollupJS and Webpack support now!)
| module.exports.init = () => initPromise; | ||
| module.exports.parse = parseCJS; |
There was a problem hiding this comment.
Note that this could be done without breaking changes:
module.exports = (source, name) => parseCJS(source, name);
module.exports.init = () => initPromise;
module.exports.parse = parseCJS;There was a problem hiding this comment.
good point. @guybedford thoughts?
There was a problem hiding this comment.
Let's keep it simple as a major change actually. Saves on type differences / leaving deprecation for subsequent work.
|
Doesn't seem like CI is set up for this repo - I didn't think to run the tests so I assume they are broken. Sorry about that @guybedford! I've put away the computer for the day, but I can probably send a PR fixing it tomorrow |
|
No problem, I added the test changes already. |
As discussed in #15.