Skip to content
This repository was archived by the owner on Jan 27, 2025. It is now read-only.

Conversation

oliverfoster
Copy link
Contributor

resolves #136

Changed

  • Allowed project to compile whilst in a node_modules folder of another project

@jlo-1
Copy link
Contributor

jlo-1 commented Aug 21, 2020

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
Copy link
Contributor Author

oliverfoster commented Aug 21, 2020

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.

@oliverfoster oliverfoster changed the title issue/136 webpack.config.js include for babel-loader issue/136 Improve webpack.config.js babel-loader directives Aug 21, 2020
@jlo-1
Copy link
Contributor

jlo-1 commented Aug 22, 2020

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 👍

@jlo-1 jlo-1 merged commit 9414a67 into uptick:master Aug 22, 2020
@oliverfoster
Copy link
Contributor Author

Ta dude.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

webpack.config.js uses both include and exclude

3 participants