Dry-view feedback

Ideas
1

This is some feedback, specially addressed to @timriley, about new changes introduced to dry-view.

First of all, let me thank you and all the dry-rb team for the great effort you are doing developing a view layer with a really well thought architecture. I think it can contribute a lot in the ruby community.

I’m using master branch in a real project, so I hope this feedback can help you. Of course, I’m going to address the aspects I would do in a different way. The other 99% is perfect in my view :smile:

  • As I already said in the repo, I fear that while we are doing dry-view more powerful, we are losing the easy to test all its components in isolation. I think this is mainly true for parts and scopes. Maybe with a bit of effort changing the way dependencies are injected to builders we could leave the users to create objects with no overloaded initializers. I haven’t thought about the actual implementation, though.

  • Sometimes I feel I need too much ceremony for simple things. Imagine I just want to define a very simple method for a view scope. Right now I have to create a new scope class and configure it. Maybe we could introduce some DSL to easy the creation of anonymous scope and context classes:

Now I’m doing:

class MyController < Dry::View::Controller
  config.scope = Class.new(Dry::View::Scope) do
    def my_tiny_method
      # ..
    end
  end
# ...
end

Maybe we could do something like:

class MyController < Dry::View::Controller
  config.scope do
    def my_tiny_method
      # ..
    end
  end
# ...
end

I think this would require some change in dry-configurable, but otherwise we could do with a lambda:

class MyController < Dry::View::Controller
  config.scope = lambda do
    def my_tiny_method
      # ..
    end
  end
# ...
end

  • There is an inconsistency in configuring scopes as classes and contexts as instances. Maybe it is due also to overloaded initializers in scopes.

  • As said in the PR adding customizable scopes:

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 find the last statement about specific to a particular controller or template (scopes!) a bit confusing about what you could already do in the controller itself. I mean, in the controller you already have the possibility to add logic particular to itself through exposures. They can be overriden when #call is invoked, but anyway they have the same purpose than custom scopes. The difference is that scopes can access the actual context in use, while a controller can only access the configured context. But maybe what we should do is change the controller so that it has access to it. Otherwise it is a bit confusing what you should put as exposure or as scope method.

Of course, my knowledge is very limited and only achieved through use and surely there are good reasons to do those things differently, but I thought that a bit of feedback could be welcomed.

Thanks again!!

1 Like
2
  • I also find a bit surprising the way to set default values for exposures. I would expect that when I set an exposure value in the controller, whether using a block or a method, that value would become the default value , while I would still be able to override it when calling #call method.

For example:

expose(:foo) { "bar" }

Here :foo will always be “bar”, even when if I do:

controller.call(foo: "overriden")

Instead, I need to do in the controller:

expose :foo, default: "bar"

Taking the two examples given in the documentation, I think we could rewrite:

  expose :users do |page: 1|
    user_repo.listing(page: page)
  end

to

expose(:page) { 1 }

expose :users do |page|
  user_repo.listing(page: page)
end

and

 expose :users, default: []

to

expose(:users) { [] }

The most important handicap I find here is not as a style issue, but as the current behaviour is breaking the principle of least surprise when a set value is not overriden with #call params.

3

I think this should be fixed now. You happy with the current arrangement, @waiting-for-dev?

At this point, beyond exposures and config, I’m hesitant to add more class-level API. I’d like to take a wait-and-see approach to see how people are using the new features (once they’re released). Obviously, you’re using them now, but I don’t want to over-optimise for a single use-case, at least not yet.

The thing that concerns me with your example, of scopes being defined inline via a block of some kind, is that we encourage people to bloat out their view classes with too many responsibilities. It also means that scopes are not reusable by default, and unit testing them becomes more indirect.

For your situation, where you still might want to define scopes inline, at the very least, you should be able to add a .scope(&block) API on your app’s base view class.

Does that sound fine for now?

This is because we expect Context objects to be pre-initialized with dependencies from across the app, since they’re meant to be one of the key integration points between the view and the app (the other being the instances of View subclasses themselves). So we want to leave the Context initialization process in the hands of the application developer, giving them a chance to inject whatever dependencies they might need.

As for the Scope classes, the initialization lifecycle for these is controlled entirely within dry-view, since they’re not intended to carry any user-injected dependencies.

So this is why config.context is meant to be an object, and config.scope is meant to be a class. Does that make sense?

Exposures are intended to provide the values that your view needs to deal with. Exposures are about data.

Scopes, on the other hand, are about providing behaviour for a particular template. e.g. you can’t “expose” a method accepting arguments. This would have to go in a scope class. This is a clear distinction, and the limitations of exposures reinforces this.

I’d be hesitant to make the context object available to exposures, because this would really muddy up what should be done and where. The fact that the context object is not available to exposures means that the user is guided towards scopes or parts for any behaviour that would rely on the context.

I think we could clear this up with some documentation. Exposures work in 2 ways:

  • Passthrough exposures, when there is no exposure block or matching instance method. In this case the matching value from the input data will be passed directly through to the template (or the default: value if none is provided)
  • “Processed” exposures (maybe we could come up with a better name), when there is a block or matching instance method provided. In this case, it’s the full responsibility of that block or method to declare which pieces of input data they want (via their params), and then the value it returns is the value passed to the template.

I feel like once the distinction between these two exposure types is made, it’s pretty clear how they should work.

You can see it pretty clearly in the Exposure#call source too:

      def call(input, locals = {})
        if proc
          call_proc(input, locals)            # <- processed exposure
        else
          input.fetch(name) { default_value } # <- passthrough exposure
        end
      end

Does that clear things up for you? I will definitely try to make this clearer in the documentation.

4

Thanks for taking the time of reviewing all of this.

:grin:

Fair enough. Your concerns make a lot of sense.

Ok, makes sense.

I don’t like that clear distinction between data and behaviour, because I like to treat functions just like data. I can define an exposure to be a lambda, and in the case I had access from the exposures to the context I could do the same with methods. In my view, this distinction is over-engineered, but surely it makes sense for a lot of people.

Hmmm… I also see it a bit over-engineered. I don’t like having two different responsibilities for a single component. I think it is unnecessarily complex, but let’s wait for the feedback of others.

5

Thanks for all your responses, @waiting-for-dev :slight_smile:

It’s becoming clear that you’re building your views quite differently to me! Is it possible for you to share some example code of the kind of views you build where you want your exposure values to act as functions?

Re: passthrough vs processed exposures:

Well, the intention was to make it feel ergonomic for both cases, which I think I’ve achieved. I’ll make an effort to document the differences clearly.

6

Sure! For example, I have the following in a custom scope because I can’t have it as exposure:

# Builds the endpoint path to sort a list of registrations.
#
# Returns a Proc which has to be invoked with the field
# to sort by. Direction is automatically detected from the
# current request path.
#
# @return [Proc] field -> path
#
# @see Llorens::Main::View::Context#sort_path
def sort_path
  context.method(:sort_path).curry.(
    path.(:registration_index, 1))
end

Then, in my template I just have to call with the field name:

<a href="<%= sort_path.(:name) %>">Name</a>
<a href="<%= sort_path.(:number) %>">Number</a>

Does it make sense now?

7

Seems like a fair enough thing to want to do. How does https://github.com/dry-rb/dry-view/pull/119 look to you? :slight_smile:

8

In my dry view, it looks very nice :smile: Thanks @timriley!

1 Like