Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Let's Replace Parcel With Webpack #2641

Merged
merged 8 commits into from Nov 30, 2021
Merged

Proposal: Let's Replace Parcel With Webpack #2641

merged 8 commits into from Nov 30, 2021

Conversation

roblarsen
Copy link
Member

Closes #2487 and #2474

This is just a development server at this point. It hot reloads HTML JS and CSS. That's enough for me. I guess we'll need a build step.

I also need to change the docs, if we go this way.

Closes #2487 and #2474

This is just a development server at this point. It hot reloads HTML JS and CSS.  That's enough for me. I'm open to contrary opinions.
@coliff coliff self-requested a review November 18, 2021 06:26
@coliff
Copy link
Member

coliff commented Nov 18, 2021

Nice! I like idea of using Webpack instead of Parcel. Some quick feedback/things I noticed:

  • the src/package.json isn't using latest versions - we should update them:
 html-webpack-plugin   ^3.2.0  →   ^5.5.0     
 webpack-cli           ^3.3.6  →   ^4.9.1     
 webpack-dev-server    ^3.7.2  →   ^4.5.0     
 webpack              ^4.36.1  →  ^5.64.1
  • the entry: in the webpack config should be app.js not main.js

  • the webpack.config.js indenting should be 2 not 4 to match .editorconfig rule and use single quotes not double quotes to match eslint config

@roblarsen
Copy link
Member Author

@coliff can you take a look at this? I want to chunk out the tasks a little bit and I'm interested in merging this part first, then look at builds for the dist folder and for the project itself.

@coliff
Copy link
Member

coliff commented Nov 24, 2021

heya. so if a user clones this repo and does npm install, then changes directory to src and runs npm run start the server won't serve the CSS because those were copied to the dist dir instead. should a user run local server from dist instead - assume so?

Unrelated note, we should remove that Place favicon.ico in the root directory comment from the HTML as it is no longer needed (the meta tag is included)

@coliff
Copy link
Member

coliff commented Nov 24, 2021

There's a couple of eslint 'errors' on webpack.config.js (when it's checked automatically with ESlint plugin in VS Code) but I assume they are fine.
image
Should be eslint ignore webpack.config.js - or add rule to turn off no-undef errors?

@roblarsen
Copy link
Member Author

@coliff Yeah, this end user webpack stuff needs to be run from dist. I'm going to write up a bunch of documentation about how the dist folder is the project for end users to make that point clear (the addition of the template repo will help reinforce that. That's one of the things that came out of that Chris Coyier article.

I'll remove the comment here.

@roblarsen
Copy link
Member Author

There's a couple of eslint 'errors' on webpack.config.js (when it's checked automatically with ESlint plugin in VS Code) but I assume they are fine. image Should be eslint ignore webpack.config.js - or add rule to turn off no-undef errors?

The errors aren't really errors, but we can turn off no-undef to make the red squiggles go away.

@coliff
Copy link
Member

coliff commented Nov 24, 2021

Great. I'll open a PR for eslint config update, we also need to update the " to ' to match the quotes: single rule in eslint itself!

@roblarsen
Copy link
Member Author

@coliff I got the eslint stuff. I added node as an environment and changed the quotes.

@roblarsen
Copy link
Member Author

I need to update the docs since right now all this does is run the development server. We reference three npm scripts- there's just one now. I will create a build task later, but I don't want to have this PR open forever.

@vltansky
Copy link
Member

Why not Vite?

@roblarsen
Copy link
Member Author

Why not Vite?

Webpack is something I use, so conceptually it was the easiest thing for me to get up and running. It also allows us to do the things that I require for this tool. It's also a giant, stable project at the heart of the web, so it seems safest.

@roblarsen roblarsen merged commit 4d9af8c into main Nov 30, 2021
@roblarsen roblarsen deleted the webpack branch November 30, 2021 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Parcel (or write a Parcel plugin to copy a list of static files)
3 participants