search

uptick / react-keyed-file-browser

Folder based file browser given a flat keyed list of objects, powered by React.
MIT License
301 stars 144 forks source link

issue/136 Improve webpack.config.js babel-loader directives #137

Closed oliverfoster closed 4 years ago

oliverfoster commented 4 years ago

resolves #136

Changed

jlo-1 commented 4 years ago

Not certain why this change is needed?

it is impossible to work on this module whilst it's being used in another project.

If you are working on react-keyed-file-browser you can use npm link so that the changes you make in your local copy are reflected in the project where it's a dependency. https://docs.npmjs.com/cli/link

oliverfoster commented 4 years ago

I'm suggesting it for sheer convenience and better usability. It isn't necessary, yes, I agree, I could use links. But for a one line change which makes it easier to work on in a wider variety of situations it just seemed sensible?

Often I setup a quick project to test out a module, editing changes locally. This line frustrated me unnecessarily.

The counter argument is: Why would you not change it, it is functionally identical and it harmonises the directives in both the sass and js parts of the compilation. It also has lower cognitive burden for anyone new who wants to contribute.

I might be missing some necessary reason as to why it is an exclude rather than an include, but this section of the config does appear to be a local directive of the compiler for this project only.

jlo-1 commented 4 years ago

I'm not the original author so not sure if there's a specific reason why it was designed this way...nonetheless, i've tested and all looks good. Given that it functions the same, follows the same syntax as the styling loaders and helps you/others out, i'm happy to merge it in.

Thanks again for contributing 👍

oliverfoster commented 4 years ago

Ta dude.