Skip to content

Replace Sprockets with Propshaft#8387

Merged
aduth merged 6 commits intomainfrom
aduth-propshaft
May 16, 2023
Merged

Replace Sprockets with Propshaft#8387
aduth merged 6 commits intomainfrom
aduth-propshaft

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented May 12, 2023

🛠 Summary of changes

This pull request proposes to replace Sprockets with Propshaft for managing the asset pipeline.

Propshaft is an officially-maintained successor to Sprockets whose goals are closer in alignment to our current asset build tooling than Sprockets, since we perform most of the asset transformations separate from Rails (source). The remaining features we rely on from Sprockets (URL remapping, filename digests) are also available in Propshaft.

Generally, the hope would be that moving to Propshaft would...

  • Improve performance (primarily in local development, specs, and asset compilation, not as much at production runtime)
  • Bring us into closer alignment to Rails future defaults
  • Avoid ongoing issues related to undesirable Sprockets default behaviors (e.g. sassc errors)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From a developer experience perspective, I actually preferred having node_modules as a load path and requiring fully-qualified paths to specific dependencies. It still works in Propshaft; however, Propshaft will copy all files from load paths into public/assets, which essentially would mean copying over the entirety of node_modules.

@aduth
Copy link
Contributor Author

aduth commented May 15, 2023

I was hoping to see a noticeable drop in GitLab build time, but it looks roughly the same as previous times. It looks like it could save around 30 seconds overall time (3-4%), though that's based on a few data points and a high margin for error.

@aduth
Copy link
Contributor Author

aduth commented May 15, 2023

This looks to work pretty well at a glance in my sandbox after the changes in 4149043.

At this point, the only concern I might have is the blurb about "beta software" in the Propshaft documentation (docs). I hadn't noticed this 'til putting together the pull request, so wondering how much appetite we'd have for running it in production.

@aduth aduth marked this pull request as ready for review May 15, 2023 17:11
@zachmargolis
Copy link
Contributor

At this point, the only concern I might have is the blurb about "beta software" in the Propshaft documentation (docs). I hadn't noticed this 'til putting together the pull request, so wondering how much appetite we'd have for running it in production.

The changes seem relatively minimal, we could always revert if we want to switch course? This doesn't feel like a super big commitment to me to try it out

@mitchellhenke
Copy link
Contributor

At this point, the only concern I might have is the blurb about "beta software" in the Propshaft documentation (docs). I hadn't noticed this 'til putting together the pull request, so wondering how much appetite we'd have for running it in production.

The changes seem relatively minimal, we could always revert if we want to switch course? This doesn't feel like a super big commitment to me to try it out

Agreed 👍🏼

aduth added 6 commits May 16, 2023 08:04
changelog: Internal, Build Tooling, Update asset pipeline to replace Sprockets with Propshaft
Propshaft can't handle proc value. Hopefully the "request is nil" issue no longer happens after transitioning
@aduth aduth force-pushed the aduth-propshaft branch from 4149043 to f586ac4 Compare May 16, 2023 12:18
@aduth
Copy link
Contributor Author

aduth commented May 16, 2023

👍 Let's give it a shot!

@aduth aduth merged commit c8d8a38 into main May 16, 2023
@aduth aduth deleted the aduth-propshaft branch May 16, 2023 12:53
aduth added a commit that referenced this pull request May 17, 2023
aduth added a commit that referenced this pull request May 17, 2023
* Revert "Fix handling for missing SP logo (#8409)"

This reverts commit e3da0a7.

* Revert "Try absolute path for Propshaft assets (#8402)"

This reverts commit 8a97e26.

* Revert "Replace Sprockets with Propshaft (#8387)"

This reverts commit c8d8a38.

* Keep spec improvements from e3da0a7

* Keep spec improvements from c8d8a38

* Keep webpack digest suffix from c8d8a38
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