bridgetownrb / bridgetown

A next-generation progressive site generator & fullstack framework, powered by Ruby
https://www.bridgetownrb.com
MIT License
1.11k stars 114 forks source link

Initial batch of swapping Active Support extensions for in-house alternatives (WIP) #881

Open jaredcwhite opened 2 months ago

jaredcwhite commented 2 months ago

Congruent with our stated goals for Bridgetown 2.0 here: https://www.bridgetownrb.com/future/road-to-bridgetown-2.0-escaping-burnout/

This PR offers an initial batch of in-house Ruby extensions in lieu of using Active Support dependencies. It also sets up a paradigm of using Zeitwerk and modules to manage core object inclusions. Update: see below for thoughts about mostly relying on refinements vs. standard monkey-patches.

Note that not all extensions will necessarily make the cut as we go through this process, but these all feel like worthwhile endeavors to me.

Feedback welcome!

render[bot] commented 2 months ago

Your Render PR Server URL is https://bridgetown-beta-pr-881.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-cod3jmv79t8c739bpeo0.

render[bot] commented 2 months ago

Your Render PR Server URL is https://bridgetown-api-pr-881.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-cod3jn779t8c739bpeu0.

ayushn21 commented 2 months ago

Is it worth separating our extensions from bridgetown-core into another gem still contained within our monorepo?

jaredcwhite commented 2 months ago

@ayushn21 Ooo, I love that idea! I can immediately think of a few other existing core utilities we could extract out as well. Could then be a nice little standalone gem.

Any ideas on what to call it?

ayushn21 commented 2 months ago

It's not the most creative name, but maybe bridgetown-ruby? That signifies we're creating a dialect under the Ruby stdlib for use within Bridgetown.

jaredcwhite commented 2 months ago

@ayushn21 Ah, hmm well that would keep the scope pretty small (and not appropriate for the utilities I was thinking about). Let me chew on that…

jaredcwhite commented 2 months ago

@ayushn21 what do you think about bridgetown-lib?

brandondrew commented 2 months ago

bridgetown-support? bridgetown-extensions?

ayushn21 commented 2 months ago

bridgetown-foundation?

brandondrew commented 2 months ago

abutment?

(An abutment is the part of a bridge that provides the support necessary to distribute the weight and stresses from the bridge to the ground, and typically anchors the bridge at either end.)

jaredcwhite commented 2 months ago

abutment?

@brandondrew I like the direction you're headed in…too bad that word immediately gets me thinking about juvenile humor… 😏😂

brandondrew commented 2 months ago

@brandondrew I like the direction you're headed in…too bad that word immediately gets me thinking about juvenile humor… 😏😂

yeah, I should have thought of that...

pablojimeno commented 2 months ago

What about Bridgetown Bridge? I think the repetition sounds great, but also the concept and the meta reference are interesting.

brandondrew commented 2 months ago

bridgetown-pillar or bridgetown-pillars

jaredcwhite commented 2 months ago

@brandondrew Or could go with bridgetown-piers — I did like 15 seconds of research and it seems the columns that hold up a bridge above water are called piers. 🤷🏻‍♂️

ayushn21 commented 2 months ago

bridgetown-piers reminds of that loathsome human Piers Morgan 😂.

I'm quite partial to bridgetown-foundation because I came up with it (obviously).

But it's also a nice parallel to the iOS/Mac platform. There you have the base language (Swift or Obj-C) and then a framework called "Foundation" which sits on top of that containing a bunch of Apple system APIs. On top of that you have the framework for the platform you're developing on (UIKit, AppKit, whatever else...).

jaredcwhite commented 2 months ago

@ayushn21 Let's go with that then. Sounds good to me!

jaredcwhite commented 2 months ago

Bit of a direction switch with the latest commit…I have been an unabashed fan of Ruby refinements for the longest time, but a while back I was sort of talked out of the idea due to the performance hit of using a refined method.

Well I'm coming back around to the idea of it being a good thing. I did some benchmarking on a String using a refined method vs a standard monkey-patched method, and standard monkey-patching was about 1.25x the performance. So the question starts to become, how often does this a refined method get called during a span of time (a build process or an HTTP request, essentially)? 1 time? 5 times? 100 times? Unless it's being called literally thousands of times (I only started to see major "real time" difference in the millions), I think we're splitting hairs. We can identify certain code paths where cumulative performance is absolutely critical (so I might back out of a few changes in that recent commit), but otherwise I think it's worth it.

"indent me!".indent(2) # NoMethodError

using Bridgetown::Refinements

"indent me!".indent(2) # "  indent me!"

is just so cool. Of course you could also just reach for individual refinements, which is as close to "imports" as you'll get in Ruby:

"indent me!".indent(2) # NoMethodError

using Bridgetown::Foundation::RefineExts::String

"indent me!".indent(2) # "  indent me!"

I'm still sussing out what we might want as core extensions globally vs. refinements. SomeClass.descendants seems like a no-brainer as a core ext—frankly it should be in Ruby proper. And starts_with? as an alias to start_with? also seems like a no-brainer. Other stuff gets murky real fast…and I'd much feel more cautious/anxious patching any Ruby core classes if I knew it was a global change which we'd have to support virtually forever and that might conflict with both third-party gems and future Rubies themselves. (Hence the very real downsides of an ActiveSupport in the first place!)

Thoughts? Questions? Ideas?

jaredcwhite commented 2 months ago

OK, this PR is fairly well baked at this point…I'll leave it open for a while for anything else which feels like low-hanging fruit, but generally it seems in good shape to pull into the v2.0 release.

jclusso commented 2 months ago

I checked this out to see if it broke anything. Trying to run a build for us results in the following error. I looked through this PR, but wasn't able to hunt down what cause this. I'm not sure if this is something with Bridgetown::Component or something we're using in ActiveSupport that was removed.

[Bridgetown] Stopping auxiliary processes...                                                                        
[Bridgetown]   Exception raised: NameError                                                                          
[Bridgetown] uninitialized constant ActionView::OutputBuffer                           
[Bridgetown]                  1: /Users/jarrett/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/actionview-7.1.3.2/lib/action_view/helpers/capture_helper.rb:49:in `capture'                                                             
[Bridgetown]                  2: /Users/jarrett/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/bundler/gems/bridgetown-e172990a669c/bridgetown-core/lib/bridgetown-core/component.rb:167:in `render' 
jaredcwhite commented 2 months ago

@jclusso Hmm…well Action View was never something we pulled into core, but that gets me wondering if you are also using ViewComponent?

jclusso commented 2 months ago

@jclusso Hmm…well Action View was never something we pulled into core, but that gets me wondering if you are also using ViewComponent?

We're just using Bridgetown::Component. We do pull in ActionView.

class SiteComponent < Bridgetown::Component
  include ActionView::Helpers::TagHelper
end
jclusso commented 2 months ago

If I follow the stack trace above it leads me to: https://github.com/bridgetownrb/bridgetown/blob/24b39fa976220abede01b3cba390a687898f2354/bridgetown-core/lib/bridgetown-core/converters/erb_templates.rb#L66

which inherits from ActiveSupport::SafeBuffer. https://github.com/bridgetownrb/bridgetown/blob/24b39fa976220abede01b3cba390a687898f2354/bridgetown-core/lib/bridgetown-core/converters/erb_templates.rb#L6

I'm not sure how any of this works really.

jclusso commented 1 week ago

If I follow the stack trace above it leads me to:

https://github.com/bridgetownrb/bridgetown/blob/24b39fa976220abede01b3cba390a687898f2354/bridgetown-core/lib/bridgetown-core/converters/erb_templates.rb#L66

which inherits from ActiveSupport::SafeBuffer.

https://github.com/bridgetownrb/bridgetown/blob/24b39fa976220abede01b3cba390a687898f2354/bridgetown-core/lib/bridgetown-core/converters/erb_templates.rb#L6

I'm not sure how any of this works really.

Were you ever able to hunt anything down related to why this breaks?

jaredcwhite commented 1 week ago

@jclusso I don't have a clear idea, but I'm wondering if you'd need to add a require for action_view/buffers — it sounds like the ActionView::OutputBuffer class isn't getting loaded.