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

Swap babel for ESBuild and upgrade to Webpack 5 #334

Merged
merged 11 commits into from
Jul 18, 2021

Conversation

ayushn21
Copy link
Member

@ayushn21 ayushn21 commented Jun 8, 2021

This is a 🙋 feature or enhancement.

I thought it'd be a good idea to swap Babel for ESBuild since it's much faster. (I don't have anything more than anecdotal experience that says its better though so happy to revert if required).

I've also reorganized the default webpack configuration by breaking a bunch of stuff out into variables which hopefully makes it cleaner and easier to read!

This needs a bit more testing before merging as well. I've verified that a new site with Sass works but just need to check configurations and a postcss website. I don't foresee any problems though.

  • Test new postcss site
  • Test webpack related bundled configurations
  • Install esbuild packages in webpack update command

Closes #204

@jaredcwhite
Copy link
Member

Ooooooooo.

(That is all. I don't have anything to add, other than, sounds awesome!)

run "bundle exec bridgetown configure purgecss"

if File.exist?("frontend/styles/index.css")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just guarding against a case where index.css doesn't exist and printing out what the user needs to include in their CSS.

Unlikely to happen but could be the case if someone created a site using sass and then converted to postcss. We don't update the CSS/JS setup when enabling postcss on an existing site as it'd be too complicated.

@@ -41,11 +41,18 @@ def self.destination_root
config.root_dir
end

protected
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot that Thor::Group runs all the public instance methods on the class. I've moved the config helper to protected now, It might, just might, have been the cause of the test suite crashes although I can't really explain why.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to do with the crashes whatsoever; but an improvement nonetheless I reckon

@ayushn21
Copy link
Member Author

Hey @jaredcwhite, I think this is good to go. Let me know if you have any feedback! Thanks :)

@jaredcwhite
Copy link
Member

I like the idea of switching to esbuild and away from babel but it's potentially a breaking change so I'd like to test it out on a few sites first. (Although I guess nobody's "required" to upgrade their existing Webpack setup.)

Would definitely be cool to get this in the next release.

@ayushn21
Copy link
Member Author

Sure, take your time. I don't anticipate it breaking any sites but you never know so best to be safe!

@ayushn21 ayushn21 changed the title Swap babel for ESBuild and other webpack config improvements Swap babel for ESBuild and upgrade to Webpack 5 Jun 17, 2021
@ayushn21
Copy link
Member Author

Upgrading to Webpack 5 turned out to be easier than I thought. Much easier than my last attempt about 6 months ago. Clearly I've learned how to tame the beast! :D

Anyway I've tacked that onto this PR since the changes were pretty small. But that does mean we need to test this quite thoroughly. I'll test it on a few of my websites in the coming days.

@ayushn21
Copy link
Member Author

I tried this on 3 of my websites and it worked just fine!

…o babel-for-esbuild

# Conflicts:
#	bridgetown-core/lib/site_template/package.json.erb
@jaredcwhite jaredcwhite added this to the 0.21.1 milestone Jul 12, 2021
@jaredcwhite
Copy link
Member

@ayushn21 Sounds great. I'll prioritize getting this into the next release (0.21.1).

@jaredcwhite
Copy link
Member

jaredcwhite commented Jul 12, 2021

@ayushn21 We probably need an upgrade guide or some kind of documentation somewhere. For example I'd like to update the BT website itself but not quite sure what to do. Or is it as easy as just running webpack update?

@ayushn21
Copy link
Member Author

Yup it's just bridgetown webpack update! ... but run bridgetown webpack setup first if you haven't got the included default config set up yet.

@jaredcwhite jaredcwhite merged commit c7e0818 into bridgetownrb:main Jul 18, 2021
@jaredcwhite
Copy link
Member

@ayushn21 Website all ready to go! I also swapped the old node-sass out for newer sass and all seems well.

@ayushn21 ayushn21 deleted the babel-for-esbuild branch July 18, 2021 19:49
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.

feat: Upgrade the default site template to Webpack 5
2 participants