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

Feature: Migrate client-participation to webpack, remove node-sass #3

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

brendanarnold
Copy link
Contributor

We are stuck with a node-sass version which will not build on ARM Macs on an Ubuntu VM. To switch to Dart Sass as this requires node 12 and above but the current version of Gulp (v3) will not run on Node > 11.

This pulll request removes Gulp as a build system and replaces it with Webpack following in the fotsteps of compdemocracy/polis#1242

It does the following ...

  • Removes Gulp build
  • Implements Webpack build
  • Vastly reduces the number of library vulnerabilities
  • Moves the required Node version to 14 and above
  • Removes the majority of browser-side globals
  • Migrates from underscore to lodash

@sirodoht
Copy link
Owner

sirodoht commented Dec 5, 2022

Awesome! From the github action error, I think it needs python 2.7 instead of 3.0+ to build. I'll explore more in the evening during the hack night session!

@metasoarous
Copy link
Contributor

Howdy gentlemen

@brendanarnold - This is fantastic! Thanks so much for working on this. We'd love to get this merged upstream to the compdem fork, and would greatly appreciate your advise and/or help. In particular, do you foresee this being a challenge based on changes to suburb, or do expect we'd be able to cherry pick easily enough?

I set up an issue (compdemocracy/polis#1575) for coordinating getting these changes in upstream. @brendanarnold Would you please chime in there about the best path forward?

@sirodoht - After some review, I can see that most of the work you have going here is great, and we'd love to get it merged in as well. But we're unlikely to get rid of docker or heroku configuration right now, and while Caddy seems interesting, we're a bit hesitant to drop nginx in it's favor without more careful consideration. So we'd probably be more interested in pulling in other work piecemeal. What's your interest in helping with this?

Thanks again!

@brendanarnold
Copy link
Contributor Author

Hey @metasoarous, I don't think it will be a big challenge, main things that maybe we would need to change for the PR for upstream ...

  • I pulled some self-hosted dependencies into js/3rdparty (e.g. d3) - upstream will probably want to keep the self-hosted dependencies as they were - Webpack can handle this with aliases
  • The S3 and the SCP uploaders were removed since Suburb doesn't use them - these probably should be re-implemented as deployment scripts separate from Webpack since it is not really part of package bundling
  • Migration from underscore to lodash is kind of hard to justify but its in there - should be able to unpick this if desired.
  • The jsHint step in the Gulpfile was not implemented - this could be added to Webpack if desired

@sirodoht
Copy link
Owner

Hey @metasoarous!

Glad you find the work interesting. I understand many of the changes are not of interest. Feel free to draw in whatever you find interesting, of course (there is no need to attribute the changes to me in any case!).

Also, I will probably not work on polis or suburb in the immediate future, so I'm not sure when I'll merge this :)

@metasoarous
Copy link
Contributor

Thank you both; Very much appreciate it!

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.

3 participants