enonic / xp

Enonic XP
https://enonic.com
GNU General Public License v3.0
202 stars 34 forks source link

pageContributions headBegin, headEnd, bodyBegin and bodyEnd should always be arrays in response object sent to filter. #5560

Closed ComLock closed 6 years ago

ComLock commented 7 years ago

You can still allow programmers to use strings in the controllers. But by the time the response object is sent to the first filter they should all be arrays.

We've even got an ugly warning in our doc about this problem: http://xp.readthedocs.io/en/stable/developer/site/contributions.html

It's really stupid to have to do something like this in every filter:

if(!response.pageContributions) {
    response.pageContributions = {
        headBegin: [],
        headEnd: [],
        bodyBegin: [],
        bodyEnd: []
    };
} else {
    const headBegin = res.pageContributions.headBegin;
    if(!headBegin) {
        res.pageContributions.headBegin = [];
    } else {
        res.pageContributions.headBegin = forceArray(headBegin);
    }
    const headEnd = res.pageContributions.headEnd;
    if(!headEnd) {
        res.pageContributions.headEnd = [];
    } else {
        res.pageContributions.headEnd = forceArray(headEnd);
    }
    const bodyBegin = res.pageContributions.bodyBegin;
    if(!bodyBegin) {
        res.pageContributions.bodyBegin = [];
    } else {
        res.pageContributions.bodyBegin = forceArray(bodyBegin);
    }
    const bodyEnd = res.pageContributions.bodyEnd;
    if(!bodyEnd) {
        res.pageContributions.bodyEnd = [];
    } else {
        res.pageContributions.bodyEnd = forceArray(bodyEnd);
    }
}
Bellfalasch commented 7 years ago

Just had issues with this during class, again (we always do). I'm very +10000 on always making pagecontributions return arrays. If not possible for 6.x then at least for 7.0

sigdestad commented 7 years ago

Won't this break backward compatibility?

ComLock commented 7 years ago

I don't think so. They can currently only be undefined, string or array. As such you always have to handle the array case when coding filters, so it would only break bad code that would already fail depending upon the input.

  1. sep. 2017 12:38 skrev "Thomas Sigdestad" notifications@github.com:

Won't this break backward compatibility?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/enonic/xp/issues/5560#issuecomment-328069983, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHQP1t5ARvp-_L3OHJtfwjqXA7LrQt-ks5sgRkMgaJpZM4PHCzZ .

Bellfalasch commented 7 years ago

I'm siding with @ComLock here. If a developer only assumes strings here, their code will break anyway. The moment item 2 is added, their code will crash. If we always sends arrays, those that already handle arrays properly (like most do) this will work out of the box. Only the old code, that would crash anyway, would crash.