feat: Expression extension framework #4372
Conversation
|
|
n8n-assistant
bot
added
core
valya
force-pushed
the
n8n-4486-implement-framework-for-extensions
branch
2 times, most recently
from
d99e7e9
to
e9448cd
Compare
valya
force-pushed
the
n8n-4486-implement-framework-for-extensions
branch
from
f1f1d99
to
6b6cffa
Compare
valya
changed the title
valya
force-pushed
the
n8n-4486-implement-framework-for-extensions
branch
from
0a64d0b
to
b5c8119
Compare
|
I don't know enough about our current expressions code to approve, but overall (besides the minor linting issues) this looks good to me. Thanks for adding all the tests. |
| throw new ExpressionError('arguments must be passed to filter'); | ||
| } | ||
| const terms = extraArgs as string[] | number[]; | ||
| return value.filter((v: string | number) => (terms as Array<typeof v>).includes(v)); |
There was a problem hiding this comment.
According to spec, we also need a flag to decide whether to keep or remove the matched keys
| function isDateTime(date: any): date is DateTime { | ||
| if (date) { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| return DateTime.isDateTime(date); |
There was a problem hiding this comment.
This doesn't really work - n8n always gets data as strings, it's never going to be an instance of DateTime. We need to check whether the string can be converted to a date instead.
There was a problem hiding this comment.
This is an internal function for this file only. It's used to check if it's a DateTime as opposed to a Date object. Anything that comes into the date functions will either be a Date or DateTime due to this check.
| function sayHi(value: string) { | ||
| return `hi ${value}`; | ||
| } |
There was a problem hiding this comment.
We can remove this function since it was used just for testing
…framework-for-extensions-new
…framework-for-extensions-new
* master: fix(core): Fixes event msg confirmations if no subscribers present (#5118) refactor(core): Diverge syntax error handling in expressions (#5124) fix(editor): Recover from unsaved finished execution (#5121) feat(editor): Executions page (#4997) docs: Update the contribution guidelines for node creation (#5120) feat: Expression extension framework (#4372) fix(editor): Fixes event bus test (#5119) refactor(TelegramTrigger Node, ShopifyTrigger Node): Standardize node triggers actions (#5117) feat(editor): Remove prevent-ndv-auto-open feature flag (#5114) refactor: On workflow deletion, cascade delete all entities associated with it (#5102) # Conflicts: # packages/editor-ui/src/Interface.ts
| }); | ||
|
|
||
| test('.toDate() should work on a string', () => { | ||
| expect(evaluate('={{ "2022-01-03T00:00:00.000+00:00".toDate() }}')).toEqual(new Date(2022, 0, 3)); |
There was a problem hiding this comment.
This test is consistently failing for me locally. Looks like a UTC/Timezone issue
There was a problem hiding this comment.
looks like this test will only pass on a system running in UTC, as new Date(2022, 0, 3) will produce a different ISO string for different timezomes.
|
Got released with |
No description provided.