Re: [UndoManager] Re-introduce DOMTransaction interface?

On Thu, Jul 5, 2012 at 1:02 PM, Ryosuke Niwa <rniwa@webkit.org> wrote:

> On Thu, Jul 5, 2012 at 12:45 PM, James Graham <jgraham@opera.com> wrote:
>
>> On Thu, 5 Jul 2012, Ryosuke Niwa wrote:
>>
>>> On Thu, Jul 5, 2012 at 8:08 AM, James Graham <jgraham@opera.com> **
>>> wrote:
>>>
>>>>       On 07/05/2012 12:38 AM, Ryosuke Niwa wrote:
>>>>>       After this change, authors can write:
>>>>>       scope.undoManager.transact(new AutomaticDOMTransaction{**function
>>>>> () {
>>>>>            scope.appendChild("foo");
>>>>>       }, 'append "foo"'));
>>>>>
>>>>>  [...]
>>
>>>
>>>>>       document.undoManager.transact(**new DOMTransaction({function ()
>>>>> {
>>>>>                // Draw a line on canvas
>>>>>            }, function () {
>>>>>                // Undraw a line
>>>>>            }, function () { this.execute(); },
>>>>>            'Draw a line'
>>>>>       }));
>>>>>
>>>>
>>>> I think this proposed API looks particularly verbose and ugly. I
>>>> thought we wanted to make new APIs more author
>>>> friendly and less like refugees from Java-land.
>>>>
>>>
>>>
>>> What makes you say so? If anything, you don't have to have labels like
>>> execute, undo, and redo. So it's less verbose. If you
>>> don't like the long name like AutomaticDOMTransaction, we can come up
>>> with a shorter name.
>>>
>>
>> I think having to call a particular constructor for an object that is
>> just passed straight into a DOM function is verbose, and difficult to
>> internalise (because you have to remember the constructor name and case and
>> so on). I think the design with three positional arguments is much harder
>> to read than the design with three explicitly "named arguments" implemented
>> as object properties.
>
>
> But the alternative is having to remember labels like execute, undo, &
> redo.
>

In your version, you need to remember the order of the arguments, which
requires you looking it up each time. If we do decide to add the
DOMTransaction constructor back, we should keep passing it a dictionary as
it's argument. Or maybe take the label and a dictionary as arguments.


> Passing in objects containing one or more non-callback properties is also
>> an increaingly common pattern, and we are trying to replace legacy APIs
>> that took lots of positional arguments with options-object based
>> replacements (e.g. init*Event). From the point of view of a javascript
>> author there is no difference between something like {foo:true} and
>> {foo:function(){}}. Insisting that there should be a difference in DOM APIs
>> because of low-level implementation concerns is doing a disservice to web
>> authors by increasing the impedence mismatch between the DOM and javascript.
>
>
> Having an explicit constructor has other advantages like making expando
> work. And we don't have to worry about authors modifying execute, undo, &
> redo after the fact because we can make them readonly (making them not
> readonly has some odd implications like the timing at which properties are
> changed matter).
>
> Also, DOM transaction object currently have two mutually exclusive
> functions executeAutomatic and execute, which define whether the
> transaction is automatic or not, and this has very weird consequences such
> as specifying both executeAutomatic and execute will result in an automatic
> transaction. Having an explicit constructor makes this interface much more
> clear and clean.
>
>  On Thu, Jul 5, 2012 at 11:07 AM, Olli Pettay <Olli.Pettay@helsinki.fi>
>>> wrote:
>>>
>>>>       We shouldn't change the UndoManager API because of implementation
>>>> issues, but if event based API ends up being
>>>>       better.
>>>>
>>>>
>>> I don't think it's reasonable to agree on an unimplementable design. In
>>> theory, mutation events can be implemented correctly
>>> but we couldn't, so we're moving on and getting rid of it.
>>>
>>
>> The current deisgn is not "unimplementable", it's just slightly more work
>> in WebKit than you would like. I don't think it's reasonable to reject good
>> designs in favour of worse designs simply because the better design isn't a
>> perfect fit for a single implementation
>>
>
> I don't think the current interface-less object is a good design
> regardless of whether it could be implemented in WebKit or not.
>
> - Ryosuke
>
>