Skip to content

Conversation

@eugenioclrc
Copy link

Just update the code an lint it with eslint, and compile it with babel.

@dogada
Copy link
Owner

dogada commented Feb 1, 2017

Thanks, Eugenio, for your pull request. It affects a lot of files and I will need more time to review it.

I like ES6 and use it in new projects but what is advantage of converting such tiny lib like RJSON is to ES2015? Will it require to include also babel-polifill to make it working in ES3 environment? Have you checked generated code with
https://www.npmjs.com/package/check-es3-syntax?

For what tasks do you use RJSON and how ES2015 migration will help you?

@eugenioclrc
Copy link
Author

Hi Dogada, i use a lot rjson.

I use rjson in the backend servers to send the data packed and to be unpacked in the user browser.

Few days ago started to work with webpack2 and wanted to try some treeshaking optimizations, something like;
import { unpack } from 'rjson';

I couldnt make it work so start read the code from rjson, to see why i couldnt do it. When i see that was write in es5 it just feel that rewriting as es6 would be fun, also rewrite the test in mocha, and added coverage report.

However i fuckup in the export (if you try the new build in the original browser test you will find out that there is no window.RJSON

I think a better and with less modifications could be to separate the pack and the unpack from de index.js, then if you just need one function you could do require('rjson/unpack') , using webpack or require('rjson/pack') if you need the pack and unpack.

I dont think it would need the babel-polifill.

I have try this cli tool;
https://www.npmjs.com/package/check-es3-syntax-cli

The only change i see was;
a.default={pack:c,unpack:d}});
For
a["default"]={pack:c,unpack:d}});

Give me some time to work on this issues and i update this pull request.

@dogada
Copy link
Owner

dogada commented Feb 4, 2017

However i fuckup in the export (if you try the new build in the original browser test you will find out that there is no window.RJSON

If you are already using webpack you can try to generate UMD build, pls see for an example: https://github.com/dogada/edux/blob/master/package.json#L29

I think a better and with less modifications could be to separate the pack and the unpack from de index.js, then if you just need one function you could do require('rjson/unpack') , using webpack or require('rjson/pack') if you need the pack and unpack.

Yeah, it's common practice nowdays to split source code on many small files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants