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

Add CLI command to update webpack config #270

Merged
merged 33 commits into from
May 24, 2021

Conversation

ayushn21
Copy link
Member

@ayushn21 ayushn21 commented Mar 31, 2021

This is a 🙋 feature or enhancement.

Summary

Adds a CLI command invoked with bridgetown webpack to manipulate a site's webpack config.

To allow us to upgrade configurations easily without overwriting our users' customizations, I've split the config into two files.

One (webpack.defaults.js) is controlled by us that shouldn't be edited by the user. The actual webpack.config.js which is used by webpack just loads the former and allows the user to override our default config options.

This way when we update the file, it won't overwrite user customizations and hopefully won't break their configs!

The 3 options available for this command are:

  • setup: Copies the webpack files into place
  • update: Updates the webpack.defaults.js file to the version contained in the bridgetown gem.
  • enable-postcss: Creates an empty postcss.config.js and swaps the sass-loader for the postcss-loader in webpack.defaults.js.

I haven't added an option to go from PostCSS to Sass as I don't think there's much of a need for it. From my point of view, I reckon more people would be converting from Sass to PostCSS than the other way round (especially since earlier versions of Bridgetown only supported Sass out of the box, and it's still the default option). Totally open to feedback though!

Docs

I've updated the "Command Line Usage" page in the docs with this new command. Since all new projects will have the two webpack files by default which also contain info about the command; I don't think we need to add anything more to the website.

However I reckon it's worth writing a blog post for existing users to upgrade to this new setup which can be easily done by running bridgetown webpack setup.

To Do

  • Remove old config file from site template
  • Copy over the new config files when a new site is created
  • Add comments to the two new config files explaining what they're for to the user
  • Add tests
  • Add docs
  • Some general misc polish
  • Fix failing tests

Context

Resolves #250

@jaredcwhite
Copy link
Member

Looking at this, definitely feels like the bridgetown.webpack.config.js file would work best as a config/webpack.defaults.js or something like that (related to your discussion point about directory structure).

@jaredcwhite
Copy link
Member

Excited to see where this is headed though! 🥳

@ayushn21
Copy link
Member Author

ayushn21 commented Apr 1, 2021

Looking at this, definitely feels like the bridgetown.webpack.config.js file would work best as a config/webpack.defaults.js or something like that (related to your discussion point about directory structure).

Yup for sure. I prefer webpack.defaults.js as a name as well, thanks! I think for this PR it's best to put that file in the root and then we can relocate that file and bridgetown.config.yml to the config folder in a separate PR.

@jaredcwhite
Copy link
Member

Sounds good 👍

@jaredcwhite jaredcwhite changed the base branch from main to 0-21-beta April 12, 2021 16:41
@jaredcwhite jaredcwhite added this to the 0.21 "Broughton Beach" milestone Apr 12, 2021
@ayushn21 ayushn21 marked this pull request as ready for review May 5, 2021 15:38
@ayushn21
Copy link
Member Author

Aaarrrggghh, I'm at my wits end with these tests .... I changed some of the test_new_command tests to not use the output logs in assertions.

Now one of the test_webpack_command tests fails, but ONLY when run with the whole suite and not by itself. 😭

@jaredcwhite
Copy link
Member

Now one of the test_webpack_command tests fails, but ONLY when run with the whole suite and not by itself. 😭

Argh, I've run into something similar in the past. Usually it's because there's some kind of state being preserved accidentally between test runs.

I'm fine with scaling back on the command-level testing and testing a few lower-level bits of logic, if you think that will help. I feel like capturing command output and verifying the text is perhaps too dicey.

@ayushn21
Copy link
Member Author

I'm utterly baffled as to why that particular test is failing! Here's what the test does:

  • Creates a new site called new-site with Bridgetown::Commands::Base.start(["new", @path])
  • Invokes the webpack command to enable postcss as such:
@cmd.inside(@full_path) do
        @cmd.invoke(:webpack, ["enable-postcss"])
end

The webpack command executes an automation to create postcss.config.js and replace webpack.defaults.js. The webpack.defaults.js template uses site.uses_postcss? to ascertain which loader to include. The uses_postcss? method just checks the site.root_dir for the existence of postcss.config.js

The reason the test is failing is that site.uses_postcss? is incorrectly returning false.

When running the full test suite, in this test for some reason, site.root_dir returns /Users/Ayush/Developer/Personal/open_source/bridgetown/bridgetown-core instead of /Users/Ayush/Developer/Personal/open_source/bridgetown/bridgetown-core/new-site.

To make things even more confusing, the webpack.rb has these these methods:

def self.destination_root
  site.root_dir
end

def site
  @site ||= Bridgetown::Site.new(configuration_with_overrides(quiet: true))
end

I put a byebug breakpoint in the enable-postcss automation and ran self.destination_root... this returned: /Users/Ayush/Developer/Personal/open_source/bridgetown/bridgetown-core/new-site

I then ran: site.root_dir ... this returned:
/Users/Ayush/Developer/Personal/open_source/bridgetown/bridgetown-core

iu-2

@ayushn21 ayushn21 merged commit 5a95144 into bridgetownrb:0-21-beta May 24, 2021
@ayushn21 ayushn21 deleted the webpack-cli-command branch May 24, 2021 21:20
jaredcwhite added a commit that referenced this pull request Jun 1, 2021
* Welcome to 0.21 "Broughton Beach"

* Highlight Render as a hosting provider

* Relations for resources (belongs_to, has_many, etc.) (#261)

* Relations for resources (belongs_to, has_many, etc)

* improve resource type comments

* Make relations available to Liquid templates

* remove stray p methods

* Add test for resource relationships

* Support pluralized belongs_to keys

* make cop happy

* Use model origin id for resource id

* Add documentation and Liquid test for resource relations

* use newer feed gem

* Use list for relation accessor examples

* Fix nil bug in relations schema

* New `Bridgetown::Component` class with a ViewComponent-ish API (#268)

* New Component class with a ViewComponent-ish API

* Isolate site in components and improve link helpers

* Remove unnecessary special render case for ViewComponent

* Fix missing variables

* Relocate markdownify to Helpers class

* Switch to new Rails-like OutputBuffer, more VC support

* Support clean render interop between builtin Component and VCs

* Add several common Liquid filters in as Ruby helpers

Also add output_safety dependency

* Additional html_safe usage

* Match Rails' capture escaping logic

* Change before_render behavior to match latest VC

* Add tests and code comments for Bridgetown::Component

* Couple of YARD updates

* fix cop offense

* Fix bug with url_for helper

* Correct failing ERB feature test

* Use new SEO and feed helpers for the website

* Fix odd cop errors

* Convert website navbar to ERB component

* fix is-active class name

* Use resource variable in default website layout

* Update documentation (WIP) on components subsystem

Also change all occurences of Javascript to JavaScript

* Add lint-html addition

* Add documentation regarding ViewComponent

* Make linthtml happy

* Fix typos

* Add link to components docs from the ERB page

* Add section on ERB output safety to the docs

* End-to-end Ruby front matter, templates, and data files (#285)

* Major feature addition for Ruby Front Matter and raw templates

Also additional work to normalize Liquid & ERB template APIs

* Improve docs

* improve error messages, support Ruby data files

* Add to_json support for Resources

* Improve code quality and test rbfm

* Refactor front matter importing and add rbfm to layouts

* Revert and use regex capture

* Remove logging around rbfm

* Lots of Ruby files and rbfm documentation, couple of fixes

* Better to_json compat

* Fix Liquid error on website build

* Fix erb render bug in docs

* Update Changelog with 0.21 beta details

* Fix dotfiles or multiple extension permalinks (resource engine) (#292)

* Fix bug which was swallowing dotfiles and multiple extensions

* Revert tag refactoring

* Remove sassify/scssify filters, html_safe the obfuscate link

* Refactor old TODOs and deprecations

* Release 0.21 beta 2

* Bugfix for `previous_resource` method

* Ensure resources with no output are still transformed

* Remove a bunch of global config accessors on site

* Switch `plugins new` command to MiniTest in repo

* Release Bridgetown 0.21.0.beta3

* Webiste: use proper metadata title for Index page

* Add I18n file reloader to the watcher process

* Add prototype page warnings of bad configs

* Add note to Resources docs about require prototype page collection key

* Add CLI command to update webpack config (#270)

* Install required packages in webpack enable postcss tool (#319)

* Add config directory and move webpack defaults into it (#316)

* Add confirmation for overwriting postcss config in tailwindcss and bt-postcss bundled configurations (#317)

* Add Bridgetown version to webpack defaults (#322)

* Resolve issue with zombie templates in Pagination/Prototype logic

* Add layout method to resource and label method to layout (#324)

* Add memoization to cache templates in `Bridgetown::Component` (#326)

* Fix the Bridgetown logger and other test improvements (#328)

* Release 0.21.0.beta4

* Release v0.21.0

Co-authored-by: Ayush Newatia <ayush@hey.com>
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: CLI command to upgrade an existing Webpack config
2 participants