Skip to content

feat: update Webpack config - #7638

Closed
yordis wants to merge 22 commits into
react:masterfrom
yordis:feature/organize-config
Closed

feat: update Webpack config#7638
yordis wants to merge 22 commits into
react:masterfrom
yordis:feature/organize-config

Conversation

@yordis

@yordisyordis commented Sep 4, 2019

Copy link
Copy Markdown

I just fixed the requested changes from #7450 since I would love to move forward with this, I can help to make it to master

cc: @iansu@petetnt@sokra@ianschmitz@mrmckeb@TheLarkInn

@yordis

Copy link
Copy Markdown
Author

Fixed merge conflicts!

@yordis

Copy link
Copy Markdown
Author

I need some help with the CI errors, I don't even know how to get to the error report from the CI

@petetnt

Copy link
Copy Markdown
Contributor

You should be able to see the errors in https://dev.azure.com/facebook/create-react-app/_build/results?buildId=356&view=logs&jobId=8999f565-280a-527f-721e-375d49cc4cd5

@heyimalex

Copy link
Copy Markdown
Contributor

My biggest issue is that it's already annoying tracking blames back across the merge that happened in #5722, so I'm kinda against re-separating them. Our webpack config is big and ugly, but most of it is essential complexity and no one is ever going to be totally happy or comfortable with it. Refactoring a big mess into two piles maybe feels good, but sometimes it just gives you two big messes you have to reason across.

All that being said, on the face the code does look a lot nicer! Maybe it would be worth it.

@yordis

Copy link
Copy Markdown
Author

@heyimalex I understand where you are coming from but worth to point out that the issue before that PR is that we had two configurations sharing nothing between them. Therefore, you had to update the configs that were shared in both files.

I think we went to the other extreme where everything is one file.

Using webpack-merge people could split what really goes into production or development mode.

I believe that this is the common ground between the two worlds.

@yordis

Copy link
Copy Markdown
Author

Hey folks, I would like your help with the failing situation since it related to eslint package

Error: Failed to load plugin 'import' declared in 'BaseConfig » /home/vsts/work/1/yarn-cache/v4/npm-eslint-config-react-app-5.0.2-alpha.0-676ad4e98b055498e0bbed90c651f736f9ba322f/node_modules/eslint-config-react-app/index.js': You cannot require a package ("eslint-plugin-import") that is not declared in your dependencies (via "/home/vsts/work/_temp/tmp.Xq1mcftxoc/test-app-pnp/__placeholder__.js")

@stale

staleBot commented Oct 13, 2019

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stalestaleBot added the stale label Oct 13, 2019
@yordis

Copy link
Copy Markdown
Author

.

@stalestaleBot removed the stale label Oct 13, 2019
@mrmckebmrmckeb added this to the 3.3 milestone Oct 17, 2019
@mrmckebmrmckeb changed the title Feature/organize configfeat: update Webpack configOct 17, 2019
@mrmckeb

Copy link
Copy Markdown
Contributor

Thanks @yordis - before doing anything else, let's hear from @iansu and/or @ianschmitz.

I think this is a candidate for 3.3.

@yordis

Copy link
Copy Markdown
Author

@mrmckeb just let me know, I am here to go back actively to fix the existing problems

@yordis
yordis requested a review from amyrlam as a code ownerOctober 25, 2019 02:42
@andriijas

Copy link
Copy Markdown
Contributor

@heyimalex I think you are on to something. Webpack 5 is moving towards even more community praised default configs so we are going to be able to clean up a bit. If we are going to start separate configs I would want to apply the same kind of thinking as we do with UI components. More lego blocks/small individual configs with separate tests to build up the full configuration.

@yordis

Copy link
Copy Markdown
Author

More lego blocks/small individual configs with separate tests to build up the full configuration.

@andriijas if you go for that route,

I hope you understand how problematic JavaScript plain objects are, and leverage #7866 in terms of webpack-chain.

Regardless,

let me know what you would like to do so I clean this up 😄

@ianschmitzianschmitz modified the milestones: 3.3, 3.4Dec 5, 2019
@iansuiansu modified the milestones: 3.4, 3.5Feb 14, 2020
@yordisyordis closed this Dec 23, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@yordis@petetnt@heyimalex@mrmckeb@andriijas@iansu@ianschmitz@facebook-github-bot@TheLarkInn