michaelboyles / redcr Goto Github PK

Describe the bug

If-else statements work, but introducing a nested if-statement will mean that the top-level else-statement is now unreachable.

Sample input

interface ArrayState {
    array: number[];
}

const foo = true;
const bar = true;

const reducer = redcr((state: ArrayState) => {
    if (foo) {
        if (bar) {
            state.array.pop();
        }
    }
    else {
        state.array.push(123);
    }
});

Current output

ES2020

const foo = true;
const bar = true;
const reducer = (state) => {
    if (foo && bar) {
        return {
            ...state,
            array: state.array.slice(0, state.array.length - 1)
        };
    }
    return state; // This line means that the else is unreachable
    if (!foo) {
        return {
            ...state,
            array: [...state.array, 123]
        };
    }
};

Describe the bug

Multiple increment and decrement operators don't combine together properly. Increment followed by decrement should no-op, but currently the second operation just gets dropped. Multiple increments or decrements in a row should accumulate.

Sample input

interface State {
    val: number;
}
const reducer = redcr((state: State) => {
    state.val++;
    state.val--;
});

Current output

Specify which ES version you're targeting: ES2020

const reducer = (state) => {
    state = {
        ...state,
        str: state.str + 1
    };
    return state;
};

There are at least 15 assignment operators (maybe TS even has a few more? not sure), like -= and *=, which are currently unsupported and end up mutably editing the original state. The only assignment operator which currently works is +=.

Describe the bug

When you push multiple elements to an array and then pop a subset of them, none of the elements are pushed correctly.

Likewise, for unshifting multiple elements and shifting one of them.

Sample input

interface NumberArrayState {
    arr: number[]
}
const reducer = redcr((state: NumberArrayState) => {
    state.arr.push(4, 5);
    state.arr.pop();
});

Current output

Specify which ES version you're targeting: ES2020

const reducer = (state) => {
    state = {
        ...state,
        arr: [...state.arr.slice(0, state.arr.length - 1)]
    };
    return state;
};

Example:

interface StringState {
    str: string;
}

const foo: number = 3;

const reducer = redcr((state: StringState) => {
    switch (foo) {
        case 1: 
            state.str = 'one';
            break;
        case 2:
            state.str = 'two';
            break;
        default:
            state.str = 'other';
    }
});

const reducer = redcr((state: NumberState) => state.num++);

This currently produces

const reducer = state.num++;

which is gibberish.

If used with a function body

const reducer = redcr((state: NumberState) => {state.num++});

then it mutably edits the state

const reducer = (state) => {
    state.num++;
    return state;
};

which is better but still wrong.

Describe the bug

When consts are used in conditions, the local variable is erased but still used within the condition.

Sample input

interface StringState {
    str: string;
}

const reducer = redcr((state: StringState) => {
    const condition = false;
    if (condition) {
        state.str = 'foo';
    }
});

Current output

ES2020

const reducer = (state) => {
    if (condition) {
        return {
            ...state,
            str: 'foo'
        };
    }
    return state;
};

Currently, using a loop in a Redcr reducer can end up mutably editing the state. For example

interface StringState {
    str: String
}
const reducer = redcr((state: StringState) => {
    for (let i = 0; i < 3; i++) {
        state.str += 'A';
    }
});

Will produce

const reducer = (state) => {
    for (let i = 0; i < 3; i++) {
        state.str += 'A';
    }
    return state;
};

It should probably work similar to how if-statements are handled, and continuously reassign the state

const reducer = (state) => {
    for (let i = 0; i < 3; i++) {
        state = {
            ...state,
            str: state.str + 'A'
        }
    }
    return state;
};

Could explore loop unrolling too if the number of iterations is a compile-time constant.

How often is a loop actually useful in a reducer though?

Describe the bug

When the result of a mutation, e.g. a unary operator, is assigned to a local variable, and that local variable is used in another mutation, e.g. assignment, the result can be that the original state is modified.

Sample input

An example in TypeScript of a reducer which produces unexpected results.

interface State {
    first: number;
    second: number;
}

const reducer = redcr((state: State) => {
    const foo = state.first++;
    state.second = foo;
});

Current output

Specify which ES version you're targeting: ES2020

const reducer = (state) => {
    state = {
        ...state,
        second: state.first++
    };
    return state;
};

In this case, state.first has been altered. The result should be something like

const reducer = (state) => {
    return {
        ...state,
        first: state.first + 1,
        second: state.first + 1
    };
};

Describe the bug

An array assignment to a specific index, followed by a call to a method such as Array.push will completely ignore the second method, and only the assignment will occur

Sample input

interface NumberArrayState {
    arr: number[]
}
const reducer = redcr((state: NumberArrayState) => {
    state.arr[1] = 123;
    state.arr.push(456);
});

Current output

Specify which ES version you're targeting: ES2020

const reducer = (state) => {
    state = {
        ...state,
        arr: Object.assign([...state.arr], { [1]: 123 })
    };
    return state;
};

As a consequence of the code which handles code paths (if conditions etc.), Redcr always ends up reassigning the parameter, regardless of whether it actually needs to. For example

interface StringState {
    str: String
}
const reducer = redcr((state: StringState) => {
    state.str = 'foo';
});

The generated code looks like this

const reducer = (state) => {
    state = {
        ...state,
        str: 'foo'
    };
    return state;
};

It would be better if it returned this

const reducer = (state) => {
    return {
        ...state,
        str: 'foo'
    };
};

Describe the bug

When using array functions as an expression lambda (no curly braces), the output is gibberish. The final reducer is not even a function.

Sample input

test('Array pop as expression arrow function', () => {
    const reducer = redcr((state: NumberArrayState) => state.arr.pop());

    const oldState: NumberArrayState = {arr: [0, 1, 2]};
    const newState = reducer(oldState);

    expect(newState).toEqual({arr: [0, 1]});
    expect(oldState).toEqual({arr: [0, 1, 2]});
});

Current output

Using ES2020

const reducer = state.arr.pop();

When executing array operations which are the inverse of another such as

interface NumberArrayState {
    arr: number[]
}
const reducer = redcr((state: NumberArrayState) => {
    state.arr.push(123);
    state.arr.pop();
});

The generated code looks like this:

const reducer = (state) => {
    state = {
        ...state,
        arr: [...state.arr]
    };
    return state;
};

Both the ...state.arr spread and the ...state spread are pointless. It would be better if Redcr was smart enough to just remove them

Describe the bug

When return; is added to the end of a block, intuitively you should expect it to make no difference to behaviour. However, in a redcr reducer, it results in the reducer returning undefined.

I can't think of a reason anyone would sensibly do this (in fact, maybe eslint would remove it?) but it low-hanging fruit so worth fixing anyway.

Sample input

interface State {
    str: string;
}
const reducer = redcr((state: State) => {
    state.str = 'abc';
    return;
});

Current output

Specify which ES version you're targeting: ES2020

const reducer = (state) => {
    state = { ...state, str: 'abc' };
    return;
    return state;
};

There are 2 returns, and the first should just be removed entirely.

Describe the bug

If multiple array functions are called on the same field, all of them except the first call are dropped on the floor. In the example below, the result is the same as if the array pop wasn't there.

This reducer should probably no-op (and maybe warn?), but currently it will push the number 123.

Sample input

interface NumberArrayState {
    arr: number[]
}
const reducer = redcr((state: NumberArrayState) => {
    state.arr.push(123);
    state.arr.pop();
});

Current output

Specify which ES version you're targeting: ES2020

const reducer = (state) => {
    state = {
        ...state,
        arr: [...state.arr, 123]
    };
    return state;
};

It might be desirable to return early within a reducer, for example

interface State {
    str: string;
}

const val = 3;
const reducer = redcr((state: State) => {
    if (val == 3) {
        state.str = 'abc';
        return;
    }
    state.str = 'def';
});

As redcr()'s signature accepts a void-returning function, it might be expected that this will work. However, the output is

const reducer = (state) => {
    if (val == 3) {
        state = { ...state, str: 'abc' };
        return; // <<< should be   return state;
    }
    state = { ...state, str: 'def' };
    return state;
};

The first branch will end up returning undefined. A workaround exists: the author can specify return state; rather than return; but maybe that's unintuitive since the function is supposed to be void-returning.

Somewhat related to #14