Formula helpers and named expressions (part 2) #192
Conversation
test/named-expressions.spec.ts
Outdated
| @@ -54,6 +56,14 @@ describe('Named expressions', () => { | |||
| }).toThrowError("Name of Named Expression '1definitelyIncorrectName' is invalid") | |||
| }) | |||
|
|
|||
| it('when adding named expression, only formulas are accepted', () => { | |||
There was a problem hiding this comment.
Why is this limitation necessary? Named expressions should accept any expression, not only formulas. Values, other named expressions, constant arrays, ranges should be supported as well.
There was a problem hiding this comment.
Note: normalizeFormula, validateFormula, calculateFormula as indicated by their names should support only formulas, those should not change. My comment is for named expressions only
There was a problem hiding this comment.
I thought that we want to focus on formulas for now, we can surely change that to accept values too
| return this.address.sheet | ||
| } | ||
|
|
||
| public get value() { |
There was a problem hiding this comment.
Should ExportedNamedExpressionChange have value alias as well?
Or we don't need newValue when we have value? It's a little bit confusing why we have both while they are always equal.
There was a problem hiding this comment.
@wojciechczerniak only for compatibility reasons (I didn't want to break the integration), I'd leave only newValue as it betters conveys intent, but the value is already there, if our current integration doesn't rely on that then I will gladly fallback to newValue only
There was a problem hiding this comment.
@jansiegel has to review the integration anyway. A lot has changed since we did it, and since we haven't released HF or HOT integration yet so we should break the compatibility if it's necessary. Better now than later or keeping it. Maintaining the unnecessary code is a chore.
Because in a moment I will introduce second class, representing changed named expression.
swistak35
force-pushed
the
rl/formula-helpers-part-2
branch
from
c4a1ee1
to
3f9301e
Compare
Since formula is not stored, temporary sounds better
not directly know that named expression is stored in some specific cell.
Context
This changes a little bit the output of
changeswhen doing CRUD operations, but the class which is returned now is compatible (in terms of API) with previously returned object.How has this been tested?
Tests
Related issue(s):
Checklist: