Consolidate JS apply and check whitelists

[amcgee]

Currently there are two implementations of whitelisting, one in files.js which includes css and json files and one in check.js which does not. As such, the behavior of d2-style js check doesn't match d2-style js apply.

These should probably both use the shared whitelist, likely excluding css and json files for now (though I think we should look into @ismay's stylelint suggestion in #17 - it can also support styled-jsx I believe).

[amcgee]

Oops, It looks like @varl changed the code under me while I was sleeping :-D

I think this is still relevant though - the --all flag now has a confusing double-meaning because it toggles staged file checking as well as non-js file checking I believe?

[varl]

Haha, yeah I'm a sneak sometimes. 🐍 I saw the same thing...

I think the --all check should be fine, but please verify my claims. I tested it in different places and looked good to me.

The staged_files and --all should be filtered using the same whitelists (only the whitelists.js one):

• https://github.com/dhis2/cli-style/blob/master/src/cmds/js_cmds/check.js#L26-L33
• https://github.com/dhis2/cli-style/blob/master/src/cmds/js_cmds/apply.js#L33-L40

    if (all) {
        codeFiles = collectJsFiles(root_dir)

https://github.com/dhis2/cli-style/blob/master/src/files.js#L19-L22

    } else if (files) {
        codeFiles = files

This is unfiltered because a user has manually passed in a list of files on the commandline... On the other hand ... Maybe we should filter this one as well.

    } else {
        const whitelist = whitelisted(whitelists.js)
        codeFiles = staged_files(root_dir).filter(whitelist)
    }

This could be written a bit more cleverly so less of the internals of files.js needs to be exported:
https://github.com/dhis2/cli-style/blob/master/src/files.js#L8-L17

[varl]

I've cleaned up the implementation a bit, it's more clear now.

[amcgee]

Nice! Approved and merged

…

On Tue, Mar 5, 2019 at 9:50 AM Viktor Varland ***@***.***> wrote: I've cleaned up the implementation a bit, it's more clear now. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA52sM3g1Icfrp7pte8PLWciYSHVn5vYks5vTq5hgaJpZM4bfDv7> .

[dhis2-bot]

🎉 This issue has been resolved in version 2.0.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀