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

Remove webpacker and cleanup unusued CSS/JS #617

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Conversation

murny
Copy link
Collaborator

@murny murny commented Aug 7, 2024

This pull requests removes Webpacker and all related code. I also cleaned up pretty much all of our CSS/JS we have in this project, as it is all unused as of now.

We currently no longer use any custom CSS/JS from our asset pipeline. As we are pulling all CSS/JS assets from UAlberta's main website or from external CDNs since we refactored our layouts in #607.

This also left a few unused shared partials which are no longer being used as well and can be removed.

With that being said, you never know what we might do in the future. So I set up the application with latest asset pipeline code from the Rails generator (using esbuild/bootstrap and sprockets), in case we decide to use asset pipeline again in the future (but this is all commented out/not being used as of now).


# See https://github.com/rails/execjs#readme for more supported runtimes
gem "execjs"
Copy link
Collaborator Author

@murny murny Aug 7, 2024

Choose a reason for hiding this comment

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

This is a javascript runtime packaged into a gem, but shouldn't be needed since we use nodejs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I honestly think we could probably remove all these image file now too? I doubt we are using them anywhere 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is just a plain HTML file.

The main reason I changed it is that we don't actually have haml gem in our project. We are getting haml from comfy mexican sofa as an implicit dependency.

But considering everything else in this project is ERB, and ERB has better support, let's just use ERB.

@@ -30,6 +30,10 @@ j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src=
<!-- End Google Tag Manager -->
<%= stylesheet_link_tag comfy_cms_render_css_path(@cms_site.id, @cms_layout.identifier) %>
<%= javascript_include_tag comfy_cms_render_js_path(@cms_site.id, @cms_layout.identifier) %>

<%# Currently CSS/JS is coming from Ualberta's main website or external CDN's. To use the asset pipeline, uncomment the next two lines of code %>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As mentioned in various places, CSS/JS is coming from Ualberta's main website or external CDNS and we no longer using the asset pipeline. But if we ever did want to use it again, then you would just uncomment these lines of code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These shared partials are dead code now. Nothing using them anymore as of #607

@@ -0,0 +1,3 @@
// NOTE: Currently all of our CSS comes from Ualberta's main website or external CDN's and this is not actually being used.
Copy link
Collaborator Author

@murny murny Aug 7, 2024

Choose a reason for hiding this comment

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

For both JS/CSS we no longer using anything from this project. So I removed all existing JS/CSS in this project which is no longer being used now.

What is here now, is I generated everything from the latest Rails generator using bootstrap/esbuild and sprockets, just in case we might want this in the future.

nnunn
nnunn previously approved these changes Aug 8, 2024
murny added 2 commits August 9, 2024 13:02
Setup application with latest asset pipeline if we decide to use asset pipeline again
Copy link
Member

@pgwillia pgwillia left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@@ -48,7 +48,7 @@ jobs:

- uses: actions/setup-node@v4
with:
node-version: 16
node-version: 18
Copy link
Member

Choose a reason for hiding this comment

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

Let's make sure @nmacgreg is aware this is part of the deployment in the release notes.

@nnunn nnunn merged commit 6f9acbb into master Aug 12, 2024
2 checks passed
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