Transpiling

[murtyjones]

Nice library.

FYI, its usage in browsers is hampered by the fact that there are some things needed transpile in older browser (let was the main thing I noticed).

I got around it by manually specifying transpiling in my webpack config, but you might consider adding some transpiling at the module level.

[boutell]

A reasonable idea, but since we don't use this in the browser, not something that I'll have the time for in-house. I've given the issue the "contribution welcome" label.

[murtyjones]

alternatively, changing the following two lines

    let rightSize = false;
    let wordUsed;

to:

    var rightSize = false;
    var wordUsed;

appears to obviate the need for transpilation.

[boutell]

Well... ES5 is pretty old at this point. I've been the first to insist on keeping compatibility with it on many things but it's really getting to the point where those who wish to support it should be running their imports through babel or something.

On Mon, Aug 27, 2018 at 9:56 AM, Marty Jones notifications@github.com wrote:

alternatively, changing the following two lines

let rightSize = false;
let wordUsed;

to:

var rightSize = false;
var wordUsed;

appears to obviate the need for transpilation.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/punkave/random-words/issues/9#issuecomment-416235039, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fZBMqzTHz6sk-8ftRfk7g6B83B14ks5uU_p7gaJpZM4WMaJt .

--

THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT P'UNK AVENUE | (215) 755-1330 | punkave.com

[CrlH]

@murtyjones Took the liberty to do a full ES6 rewrite, added Babel while at it. Can you check this branch does the trick?

[boutell]

Thanks for giving it a go.

I have concerns though. I still see lots of "var", not sure what was accomplished by the rewrite. "var fnName = function fnName" is an unnecessary construct and it breaks top-down programming, which has never stopped being a best practice. I think readable code got replaced with a less readable reshuffling and I don't see any particular improvement as a result.

On a more minor note, eslintrc extends something that sounds in-house to me:

module.exports = { extends: "airbnb", parser: "babel-eslint"};

On Tue, Oct 2, 2018 at 9:05 AM Carl H. notifications@github.com wrote:

@murtyjones https://github.com/murtyjones Took the liberty to do a full ES6 rewrite, added Babel while at it. Can you check this branch https://github.com/CrlH/random-words/tree/feature/es6-rewrite does the trick?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/punkave/random-words/issues/9#issuecomment-426265753, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fX3UqQCmN5M3hp7p0_v1v0i2Dtrjks5ug2SggaJpZM4WMaJt .

--

Thomas Boutell, Chief Software Architect P'unk Avenue | (215) 755-1330 | punkave.com

[murtyjones]

I've forked this library so I'm going to go ahead and unsubscribe from this issue, after noting the following:

1. This library is about 90 lines of code.
2. 87 of these lines appear to be ES5
3. three of thee lines appear to be ES6:

   let rightSize = false;
   let wordUsed;
   ...
   options.formatter = (word) => word;

4. These three lines could be converted to ES5 like so:

   var rightSize = false;
   var wordUsed;
   ...
   options.formatter = function (word) { return word; };

[CrlH]

@murtyjones Indeed, but since it can be used in non-browser environment, I rewrote to ES6 anyway, transpiling it. Overkill maybe, I just liked the idea. :-)

Regarding the code:

• The rewrite is the src folder, dist is built by Babel
• I’ve moved some functions because there’s no need to expose/export them
• AirBNB ESLint config is indeed maintained by AirBNB, but well respected as a code styling guideline.

[boutell]

Use of the airbnb config is fine as long as it's something that the dependencies are enough to install/discover/whatever; that's what I'm asking about.

I was looking in the wrong place, yes. Still not sure why using "const" for functions and thus requiring them to come before the higher-level functions that call them is an improvement, though.

Fork it if you want I guess (not that I can stop you (: )