-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Extend date functions clobbering plus/minus #5170
Conversation
function minus(date: Date | DateTime, extraArgs: unknown[]): Date { | ||
function minus(date: Date | DateTime, extraArgs: unknown[]): Date | DateTime { | ||
if (isDateTime(date) && extraArgs.length === 1) { | ||
return date.plus(extraArgs[0] as DurationLike); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't that be 'minus'? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. I guess that's what I get for trying to quickly push out a fix on a Monday morning with no coffee 🤦
@@ -161,7 +162,11 @@ function isWeekend(date: Date): boolean { | |||
return [DAYS.saturday, DAYS.sunday].includes(DateTime.fromJSDate(date).weekday); | |||
} | |||
|
|||
function minus(date: Date | DateTime, extraArgs: unknown[]): Date { | |||
function minus(date: Date | DateTime, extraArgs: unknown[]): Date | DateTime { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to remain consistent with the rest of the UI, shouldn't the functions always return primitives such as strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could but if things are strung together it's going to be a lot of checking if the string is a timestamp and converting it to a date, just to turn it back into a string.
Got released with |
* master: (21 commits) 📚 Update CHANGELOG.md and main package.json to 0.211.1 🔖 Release n8n@0.211.1 ⬆️ Set n8n-core@0.151.1, n8n-editor-ui@0.177.1, n8n-nodes-base@0.209.1 and n8n-workflow@0.133.1 on n8n 🔖 Release n8n-editor-ui@0.177.1 ⬆️ Set n8n-design-system@0.51.1 and n8n-workflow@0.133.1 on n8n-editor-ui 🔖 Release n8n-design-system@0.51.1 🔖 Release n8n-nodes-base@0.209.1 ⬆️ Set n8n-core@0.151.1 and n8n-workflow@0.133.1 on n8n-nodes-base 🔖 Release n8n-node-dev@0.90.1 ⬆️ Set n8n-core@0.151.1 and n8n-workflow@0.133.1 on n8n-node-dev 🔖 Release n8n-core@0.151.1 ⬆️ Set n8n-workflow@0.133.1 on n8n-core 🔖 Release n8n-workflow@0.133.1 fix: Extension deep compare not quite working for some primitives (#5172) feat(editor): Supress validation errors for freshly added nodes (#5149) test: Update unit tests to remove hash (#5152) feat(Google Ads Node): Update api version to v11 (#4427) fix: Extend date functions clobbering plus/minus (#5170) fix: Build `cli` to fix Postgres and MySQL test runs (#5171) feat(Google Drive Trigger Node): Use resource locator component (#5148) ... # Conflicts: # packages/editor-ui/src/components/NodeCredentials.vue # packages/editor-ui/src/components/ParameterInputList.vue
Github issue / Community forum post (link here to close automatically):