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

Introduce customizable scope objects #90

Merged
merged 10 commits into from Dec 18, 2018

Conversation

Projects
None yet
4 participants
@timriley
Copy link
Member

timriley commented Dec 8, 2018

This PR introduces a new, user-customisable concept to dry-view: scopes.

Until now, dry-view has given the users the following places to provide data and behaviour to the view:

  • exposures
  • view parts, which add view-specific behaviour to the values provided by exposures
  • the context object, which provides a global, baseline rendering environment (available to the view parts, the template, and all partials within a single rendering, and in normal cases, this is also global across view controllers as well, since the same context class is usually reused across whole sections of an app)

This has been enough to cover many use cases, but it still left a gap: what if you wanted to provide some extra behaviour to particular templates, e.g. those within a single view controller rendering only, or even just to specific partials?

For this, we now have scopes. Scopes are just that - the rendering environment for a template. They consist of locals, the context, as well as the other internal objects used for rendering, etc.

You can create scopes from within templates as well as parts. When creating a scope, you pass a name (optional), as well as a hash of locals.

Scopes are created via a ScopeBuilder, which like the PartBuilder, can be configured to infer a scope class from its name, searching within a given namespace.

A scope can also be assigned to a view controller config itself, which provides the scope for the controller's base template.

To demonstrate the usefulness, let's run through a couple of concrete examples.

Adding extra behavior to a base template

template:

== hello

ruby code:

    module Test
      class ControllerScope < Dry::View::Scope
        def hello
          "Hello #{_locals[:text]}!"
        end
      end
    end

    vc = Class.new(base_vc) do
      configure do |config|
        config.template = "custom_view_controller_scope"
        config.scope = Test::ControllerScope
      end

      expose :text
    end.new

    expect(vc.(text: "world").to_s).to eq "Hello world!"

Providing default values for partials

This also demonstrates how a scope can be rendered without an explicit partial name, if the scope's name matches the partial's name.

template:

== scope(:greeting).render

_greeting partial:

| Greeting: #{greeting}

ruby code:

    module Test::Scopes
      class Greeting < Dry::View::Scope
        def greeting
          _locals.fetch(:greeting) { "Howdy" }
        end
      end
    end

    vc = Class.new(base_vc) do
      configure do |config|
        config.scope_namespace = Test::Scopes
        config.template = "named_scope_with_defaults"
      end

      expose :text
    end.new

    expect(vc.().to_s).to eq "Greeting: Howdy"

Both of these examples, while obviously simplified here, are derived from real issues we had in using dry-view across multiple complex client projects.

I'm very excited to see the addition of scopes, because it means we now have an appropriate place for every kind of view behaviour, whether it's global (context!), specific to a particular value (parts!) or specific to a particular controller or template (scopes!).

I still have some tidying to do before this PR is ready to merge, but I wanted to write up some brief thoughts on how this all works to seek some feedback at a feature/behavioural-level. Would love to hear what you think.


TODO:

  • Merge renderer, context, part_builder, scope_builder together into some kind of "Rendering" object (we're passing some or all of these around in groups and I feel it's calling out for a refactoring)
  • Consider how we handle implicit rendering when a scope's name is actually a class
  • Add specs demonstrating class being provided as scope name

@timriley timriley force-pushed the custom-scopes branch 6 times, most recently from 9513050 to 4edc05c Dec 8, 2018

@timriley

This comment has been minimized.

Copy link
Member Author

timriley commented Dec 11, 2018

Pinging @solnic @jodosha and @AlfonsoUceda to take a look at this one.

@jodosha
Copy link
Member

jodosha left a comment

@timriley I really like the idea.


The most important bit is to communicate properly the following lines:

I'm very excited to see the addition of scopes, because it means we now have an appropriate place for every kind of view behaviour, whether it's global (context!), specific to a particular value (parts!) or specific to a particular controller or template (scopes!).

That means to clarify the three levels (global, local, value) of intervention for a user, and the corresponding tools (context, scope, and parts).

Speaking of which, what's the precedence of these tools? 1. Part 2. Scope 3. Global?

@timriley

This comment has been minimized.

Copy link
Member Author

timriley commented Dec 14, 2018

@jodosha I'm really glad to hear like the approach. I agree 100% that a big part of helping people get on board with dry-view is clearly articulating each of these facilities and the purpose for each (along with some example cases, perhaps, to make things clearer). I'll be working on user docs once I've got through most of the 1.0 issues.

The order of precedence when calling a method within a template is:

  1. Methods explicitly defined on the scope object
  2. A local of that name (i.e. returning a view part)
  3. A public method defined on the context object
@timriley

This comment has been minimized.

Copy link
Member Author

timriley commented Dec 14, 2018

@solnic @flash-gordon @GustavoCaso If you have a chance, I'd love your feedback on the refactoring I've just pushed in 91c0b3d.

The background for this is that now we're adding support for custom "scope" classes, we've added a scope_builder to go along with our existing part_builder, and then because I want to to be possible to build scopes from within part classes, that scope_builder also needs to be provided to the part_builder when it's getting initialised. And then, when initializing part classes, we were ending up having to pass a whole list of related objects (renderer, context, scope_builder, part_builder). That's too many things. It was getting out of control.

So to tidy up, I've introduced a Rendering object. This is created at the beginning of the #call process on a view controller. It holds all those 4 things together, and provides an API to cover all the things we want to do (from anywhere) during the rendering process: rendering templates and partials, as well as building scopes and parts. With this in place we only have to pass a single object around to the various parts of our system. It feels much, much tidier, and it's simplified a lot of code.

However, the reason why I'd like some feedback here is that the Rendering class sets up some cyclic dependencies. When it is initialised, it passes itself to the context, scope_builder, and part_builder objects by calling #for_rendering(self) on each. Each of them then holds a reference to the rendering object so they can pass on the whole rendering environment to the objects they're vending (e.g. the part builder passes the rendering object to any of the parts it creates, so those parts can then offer a #scope method, which internally calls _rendering.scope).

I feel this approach is OK given the interrelatedness of these objects, and it's certainly helped tidy up our internals here. But if you think this kind of arrangement is a red flag, I'd love to hear your thoughts on how else we could approach it.

@timriley timriley changed the title [WIP] Introduce customizable scope objects Introduce customizable scope objects Dec 16, 2018

@GustavoCaso

This comment has been minimized.

Copy link
Member

GustavoCaso commented Dec 16, 2018

@timriley I have check the refactor you mentioned in 91c0b3d

I see no issue with it, it actually pushes the logic one level down leaving the controller class much cleaner and easier to understand 👏 👏

Id true that

def for_rendering(rendering)
  return self if rendering == self.rendering

  self.class.new(namespace: namespace, rendering: rendering)
end

is not a very usual pattern in ruby, but as you say we need the rending because context, part and scope use the method defined in rendering

One question:

While we are refactoring I noticed that we have a bunch of private class methods in controller.rb but they are truly public for people to use them:

# @api private
def self.rendering(format: config.default_format, context: config.default_context)
  Rendering.prepare(renderer(format), config, context)
end

# @api private
def self.renderer(format)
  renderers.fetch(format) {
    renderers[format] = Renderer.new(paths, format: format, **config.renderer_options)
  }
end

# @api private
def self.renderers
  @renderers ||= {}
end

# @api private
def self.exposures
  @exposures ||= Exposures.new
end

Is there something stopping us from using private_class_method and make them private?

timriley added some commits Nov 22, 2018

Introduce customizable scopes
Scopes provide a custom rendering context for a specific template or partial
Bring various objects used for rendering into single Rendering object
This provides a unified, coherent API for rendering facilities (such as rendering templates or partials, as well as creating scopes and parts), and makes it easy to pass these interrelated objects around in one go, rather than having to manage them all individually.
Move rendering builder into class method
This will be helpful when we want to prepare a standalone rendering object for testing purposes

@timriley timriley force-pushed the custom-scopes branch from c110e96 to a5c1b6a Dec 17, 2018

@timriley

This comment has been minimized.

Copy link
Member Author

timriley commented Dec 17, 2018

@GustavoCaso Thanks for taking a look. I actually have tentative plans to make .rendering public, since it'll help with testing. I suppose it wouldn't hurt to try and make the others truly private — will look at that later on.

timriley added some commits Dec 17, 2018

@timriley timriley merged commit 4acab57 into master Dec 18, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@timriley timriley deleted the custom-scopes branch Dec 18, 2018

@waiting-for-dev

This comment has been minimized.

Copy link
Contributor

waiting-for-dev commented Jan 11, 2019

Late to the party, but I'd like to add a comment.

I'm using a custom scope with a method which just curries another method defined in the context. The important thing here is that I'm not calling the context method, I'm just referencing it. So, in order to reference it I have to access the context object, where it is defined:

_rendering.context.method(:my_context_method).curry.(:one_arg)

Using _rendering.context feels quite private. I'm wondering what do you think about having a clearly public method context to have direct access to the context instance.

I know it is not a very common scenario, but still with a functional approach it makes a lot of sense having a generic method in the context specialized in custom scopes through currying.

WDYT @timriley ?

@waiting-for-dev

This comment has been minimized.

Copy link
Contributor

waiting-for-dev commented Jan 11, 2019

Of course, I could do it myself through a PR.

waiting-for-dev added a commit to waiting-for-dev/dry-view that referenced this pull request Jan 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment