Skip to content

Move the webpacker config override to @decidim/webpacker#8158

Merged
leio10 merged 1 commit intodecidim:developfrom
mainio:refactor/webpacker-style-imports
Jun 25, 2021
Merged

Move the webpacker config override to @decidim/webpacker#8158
leio10 merged 1 commit intodecidim:developfrom
mainio:refactor/webpacker-style-imports

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Jun 24, 2021

🎩 What? Why?

This moves the configuration override for dynamic stylesheet includes implemented at #8115 to the @decidim/webpacker package.

This idea came from @ferblape's comment at #8115 (comment) where he mentioned this file might change during upgrades. At that point it wasn't possible to do but after #8121 was merged it is now possible to move these to the Decidim NPM packages.

We still need to override the import at base.js file but this decouples the configuration overrides from that file to the @decidim/webpacker NPM package, so the changes needed at base.js are much more minimal.

📌 Related Issues

Testing

  • Re-create the development app
  • Run ./bin/webpack-dev-server at the development app
  • Go to the help page of the development instance
  • Make a change at some dynamically included stylesheet, e.g. decidim-budgets/app/packs/stylesheets/decidim/budgets/_budgets.scss -> add p{ color: #f00; }
  • Wait for webpacker to recompile the stylesheet
  • Take a look at the help page whether you can see the change

📋 Checklist

  • 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.

Copy link
Copy Markdown
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for taking care of this! 👏🤩

@leio10 leio10 merged commit 3e2d743 into decidim:develop Jun 25, 2021
@ahukkanen ahukkanen deleted the refactor/webpacker-style-imports branch June 25, 2021 15:13
@leio10
Copy link
Copy Markdown
Contributor

leio10 commented Jun 28, 2021

Hi @ahukkanen, this PR is breaking our staging app, do I have to add anything to the app to make it work?
These are the errors that we got:

       SassError: Can't find stylesheet to import.
         ╷
       6 │ @import "!decidim-style-imports[app]";
         │       

...

       SassError: Can't find stylesheet to import.
          ╷
       22 │ @import "!decidim-style-imports[admin]";
          │       

@ahukkanen
Copy link
Copy Markdown
Contributor Author

@leio10 I think it is #8154 that is breaking it.

You need to update this:
https://github.com/codegram/decidim-staging/blob/main/config/webpack/base.js

To match this:
https://github.com/decidim/decidim/blob/develop/decidim-core/lib/decidim/webpacker/webpack/base.js

@leio10
Copy link
Copy Markdown
Contributor

leio10 commented Jun 28, 2021

Thanks @ahukkanen, it worked! ❤️

entantoencuanto added a commit that referenced this pull request Jun 29, 2021
* develop: (47 commits)
  New Crowdin updates (#8150)
  Move the webpacker config override to @decidim/webpacker (#8158)
  Fix admin stylesheet dynamic imports (#8154)
  Fix session timeout conflicting with remember me (#7467)
  Allow to create online meetings without an URL (#8152)
  Fix verification route issues (#8146)
  Fix dont save timeout path to session (#8142)
  Fix access to import CSV results in accountability (#8132)
  Fix user report notification reported user name (#8130)
  Allow users to comment and delete their own comments (#8072)
  New Crowdin updates (#8124)
  Fix webpacker issues (#8136)
  Add comments in participatory space presentation page stats block (#8034)
  Split NPM dependencies to more granular packages (#8121)
  Metric is not shown when value is zero for blocked and reported users (#8117)
  Add missing templates translations (#8133)
  Add missing translation for authorization_modals (#8129)
  Polls in meetings (#8065)
  Fix flaky test on initiatives (#8128)
  Filter participants admin (#8104)
  ...
@andreslucena andreslucena added type: internal PRs that aren't necessary to add to the CHANGELOG for implementers and removed type: enhancement target: developer-experience labels Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in-review module: core type: internal PRs that aren't necessary to add to the CHANGELOG for implementers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants