Skip to content

Migrate to Webpacker (CSS and images)#7733

Merged
mrcasals merged 268 commits intodevelopfrom
7291-migrate-to-webpacker-css
May 5, 2021
Merged

Migrate to Webpacker (CSS and images)#7733
mrcasals merged 268 commits intodevelopfrom
7291-migrate-to-webpacker-css

Conversation

@ferblape
Copy link
Copy Markdown
Contributor

@ferblape ferblape commented Mar 25, 2021

(This PR is built on top of #7464)

This PR migrates CSSs and images from the old and beloved Asset Pipeline to they young and modern Webpacker.

This PR uses a beta version of Webpacker 6 gem, which is almost ready to be released and uses the modern Webpack v5. At the time of creating this PR that version hasn't been released yet.

Development process

The development process is very smooth, in the root folder of Decidim the webpack-dev-server is available via bin/webpack-dev-server and the development_app is configured to use Decidim's Webpacker, so asset changes are detected and automatic reloading is enabled.

Testing environment

Node has been added to all github workflows, together with a yarn install (using a cache folder to speedup) and it's necessary to precompile the assets before running the specs.

Documentation

Some documents have been added:

📌 Related Issues

Testing

You should create a new development_app, start webpack dev server and start browsing:

  • you should see packs compiled in development_app/public/packs
  • if you inspect the source code you should see the new packs
  • if you change any css you should see the page reloading
  • and of course everything should work as before!

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

ferblape added 6 commits May 3, 2021 07:18
…ontent

Because of the name of the file was based in the hash, there are some SVGs that are duplicated (i.e decidim_proposals, decidim_elections) and those weren't generated by the first compilation of Webpack
Copy link
Copy Markdown

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 49746 lines exceeds the maximum allowed for the inline comments feature.

Copy link
Copy Markdown

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 49747 lines exceeds the maximum allowed for the inline comments feature.

Copy link
Copy Markdown

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 49747 lines exceeds the maximum allowed for the inline comments feature.

Copy link
Copy Markdown

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 49748 lines exceeds the maximum allowed for the inline comments feature.

@ferblape
Copy link
Copy Markdown
Contributor Author

ferblape commented May 3, 2021

@mrcasals @ahukkanen sorry for the delay, now the branch is updated and working again. I had a hard time figuring out why some of the SVGs weren't being compiled by Webpack, until I realized that the path for the compiled assets is [hash][extension], so when the hash is duplicated the file is not generated, and some of the Decidim logos use the same SVG, and thus those duplicated logos were missing, causing some tests to fail, of course.

On the other side, I created the task decidim:webpacker:install which moves all the config necessary to run webpacker inside the rails app and works pretty well. Please take a look, I think it's ready to be reviewed.

I also upgraded as we agreed the version of node, and updated some dependencies thanks to npm audit command.

Once you confirm it's ok I'd like to udpate the PR description and the docs explaining a bit how it works and how can new JS dependencies be added to Decidim.

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented May 5, 2021

It's super difficult to review, so I'll merge and pray everything's fine 😅

@mrcasals mrcasals merged commit 1b131d3 into develop May 5, 2021
@mrcasals mrcasals deleted the 7291-migrate-to-webpacker-css branch May 5, 2021 12:15
@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented May 5, 2021

@ferblape for future reference, don't wait for me before adding documentation, you can write it in another PR and specify that it requires this PR in order to be merged 😄

@ferblape
Copy link
Copy Markdown
Contributor Author

ferblape commented May 5, 2021

PR updated with the links to the docs

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check webpacker branch in existing Decidim apps Document apps and modules migration to Webpack(er) Migrate to webpacker

4 participants