Replace the async.waterfall in index with a Promise chain

[mzgoddard]

• Update commitlint dependencies
  The commitmsg command was erroring and so not letting me commit changes.
• Replace async.waterfall with Promise chain
  scratch-parser is the only scratch package used by scratch-gui using the async module. We can save ~180KB of javascript by either using async-es, another similar package or promises. I didn't want to rewrite the commonjs modules into es modules so I opted for Promises.

[kchadha]

This new structure seems a bit awkward now, maybe we can reorganize this code so that we have nested .thens? E.g. the other functions e.g. parse, validate, etc. live inside the .then for the unpack and then the last bit has access to the unpackedProject and can call the callbacks...

[mzgoddard]

??

I think what you described is nesting thens.

Promise.all([
    toUnpack
        .then(function (unpacked) {
            return parse(unpacked[0])
                .then(validate.bind(null, isSprite));
        }),
    toUnpack
        .then(function (unpacked) {
            return unpacked[1];
        })
])
    .then(callback.bind(null, null), callback);

How about:

unpack(input, isSprite)
    .then(function (unpacked) {
        return Promise.all([parse(unpacked[0]), unpacked[1]]);
    })
    .then(function (parsed) {
        return Promise.all([validate(isSprite, parsed[0]), parsed[1]]);
    })
    .then(callback.bind(null, null), callback);

I normally try to write promise functions that do not need external contexts. It probably doesn't matter here since it needs so few lines, but its a big help with large promise chain functions.

On that point what about:

unpack(input, isSprite)
    .then(function (unpackedProject) {
        return parse(unpackedProject[0])
            .then(validate.bind(null, isSprite))
            .then(function (validatedProject) {
                return [validatedProject, unpackedProject[1]];
            });
    })
    .then(callback.bind(null, null), callback);

[kchadha]

@mzgoddard, that last one is pretty much what I was trying to suggest. Sorry I should have been more clear.

[kchadha]

I think the code can probably be re-written to be a little easier to understand. I left a comment in the code review.

[kchadha]

@mzgoddard should I be seeing an update to commitlint in the changed files? You mentioned it in the PR description but I don't see the related change..

[mzgoddard]

@kchadha I believe I removed the commitlint thing in the https://github.com/LLK/scratch-parser/compare/82da89870420c1e83d96e9dab64d19d805cd7c78..593e12333881761e93b41a831f9a828d276fa9cc force push. The commitlint change was in develop so I rebased and and was able to drop that change.

[mzgoddard]

• Change waterfall promise to one of the discussed possibilities

[kchadha]

@mzgoddard, thanks for making those changes. LGTM

[kchadha]

🎉 This PR is included in version 4.3.6 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀