refactor: Use `NodeConnectionType` consistently across the code base (no-changelog) #10595

RicardoE105 · 2024-08-28T17:06:29Z

Summary

I was moving the NodeSettings.vue file #10545 to use the composition API and trying to fix one small type issue snowball into a lot of type issues because we have two types in the FE that represent the same. One is an enum, and the other is a type union that we use in the editor-ui, workflow, and nodes-base, and they do not match (a known problem with enums). So, to avoid this, I deleted the ConnectionType type and used the NodeConnectionType enum everywhere.

Uses NodeConnectionType consistently across the code base.
Deletes ConnectionType type.

ALL unit, integration and e2e tests pass.

Review / Merge checklist

PR title and summary are descriptive. (conventions)
Docs updated or follow-up ticket created.
Tests included.
PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

cypress · 2024-08-28T17:37:14Z

n8n Run #6665

Run Properties: Passed #6665 • 93a6afacd5: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 RicardoE105 🗃️ e2e/*

Project	`n8n`
Branch Review	`refactor-types-NodeConnectionType`
Run status	`Passed #6665`
Run duration	`04m 48s`
Commit	`93a6afacd5: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 RicardoE105 🗃️ e2e/*`
Committer	`Ricardo Espinoza`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`421`
View all changes introduced in this branch ↗︎

RicardoE105 · 2024-08-28T17:53:24Z

packages/@n8n/nodes-langchain/nodes/memory/MemoryRedisChat/MemoryRedisChat.node.ts

@@ -155,6 +155,7 @@ export class MemoryRedisChat implements INodeType {

 		const memory = new memClass({
 			memoryKey: 'chat_history',
+			//@ts-ignore


Only ts-ignore I had to add. Very weird because I did not touch anything related to this. Wondering if @lanchain/core updated its types 🤔

Could this be related to the changes in pnpm-lock.yaml? I guess it shouldn't have changed since there are no package.json changes?

It seems like this was the case! Updated the pnpm-lock.yaml to have we have in master and it seems to have fixed the type issue 🙏

tomi

Thank you for doing this improvement ❤️ Certainly wasn't a small feat. Mostly looking great 👌 Couple questions and also we shouldn't remove more specific types, or was there a reason for doing that?

tomi · 2024-08-29T07:21:55Z

packages/@n8n/nodes-langchain/nodes/memory/MemoryRedisChat/MemoryRedisChat.node.ts

@@ -155,6 +155,7 @@ export class MemoryRedisChat implements INodeType {

 		const memory = new memClass({
 			memoryKey: 'chat_history',
+			//@ts-ignore


Could this be related to the changes in pnpm-lock.yaml? I guess it shouldn't have changed since there are no package.json changes?

tomi · 2024-08-29T07:24:53Z

packages/nodes-base/.eslintrc.js

@@ -66,13 +66,13 @@ module.exports = {
 				'n8n-nodes-base/node-class-description-display-name-unsuffixed-trigger-node': 'error',
 				'n8n-nodes-base/node-class-description-empty-string': 'error',
 				'n8n-nodes-base/node-class-description-icon-not-svg': 'error',
-				'n8n-nodes-base/node-class-description-inputs-wrong-regular-node': 'error',
+				'n8n-nodes-base/node-class-description-inputs-wrong-regular-node': 'off',


Why are these two turned off?

This rules check if the output or input properties are and array of string with valid values. With the enum they do not work, we need to update the rule or get rid of it altogether! Will double check with Ivan as he created this rules

Synced with Ivan and suggested to leave them off until we update the rules validate the enums instead of an array of strings

Alright, can you create a follow-up for that so we don't forget 🙏

packages/nodes-base/nodes/Bubble/GenericFunctions.ts

packages/nodes-base/nodes/Freshservice/GenericFunctions.ts

packages/nodes-base/nodes/Google/Firebase/RealtimeDatabase/GenericFunctions.ts

packages/nodes-base/nodes/Grist/GenericFunctions.ts

packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts

tomi

🚀

github-actions · 2024-08-29T13:34:56Z

✅ All Cypress E2E specs passed

* master: (21 commits) feat: Add queue mode setup to benchmarks (no-changelog) (#10608) feat: Add n8n postgres setup to benchmarks (no-changelog) (#10604) fix(API): Update express-openapi-validator to resolve AIKIDO-2024-10229 (#10612) fix: Fix edge case in log in (no-changelog) (#10610) feat: Add local orchestration of benchmarks (no-changelog) (#10589) ci: Run nightly benchmark against nightly n8n image (no-changelog) (#10588) fix: Reduce variability in benchmarks (no-changelog) (#10606) docs: Add missing changelog entry (#10609) refactor(editor): Convert ResourceLocator to composition API (no-changelog) (#10526) feat(editor): Update new canvas node handle label rendering mechanism and design (no-changelog) (#10611) refactor(editor): Convert credential related components to composition API (no-changelog) (#10530) fix(HTTP Request Node): Sanitize authorization headers (#10607) refactor: Use `NodeConnectionType` consistently across the code base (no-changelog) (#10595) fix(editor): Hide execution buttons in readonly mode in new canvas (no-changelog) (#10603) fix(editor): Prevent keyboard shortcuts when ndv is open in new canvas (no-changelog) (#10601) fix(editor): Add confirmation toast when changing user role (#10592) feat(editor): Add support for changing sticky notes color in new canvas (no-changelog) (#10593) ci: Fix `forceConsistentCasingInFileNames` for aliased paths (no-changelog) (#10598) feat(editor): Allow sticky notes alongside fallback nodes in new canvas (no-changelog) (#10583) ci: Push nightly images to ghcr (no-changelog) (#10580) ...

janober · 2024-09-05T10:42:25Z

Got released with n8n@1.58.0

anantanandgupta · 2024-09-12T09:51:22Z

eslint-plugin-n8n-nodes-base is not happy with NodeConnectionType.Main

30:3 error Replace with "['main']" [autofixable] n8n-nodes-base/node-class-description-outputs-wrong

if I put ['main'] then INodeTypeDescription yells at me.

I am not sure how the community node developers are keeping up with you guys. :(

Juneezee

#10595 (comment)

eslint-plugin-n8n-nodes-base is not happy with NodeConnectionType.Main

30:3 error Replace with "['main']" [autofixable] n8n-nodes-base/node-class-description-outputs-wrong

if I put ['main'] then INodeTypeDescription yells at me.

I am not sure how the community node developers are keeping up with you guys. :(

This caused TypeScript and ESLint errors for existing users of n8n-workflow package:

tomi · 2025-01-07T13:16:35Z

@Juneezee apologies for the harm this caused. Us not realizing this is a breaking change for the n8n-workflow package was an oversight from our end. To fix the errors you can use the NodeConnectionType enum from the n8n-workflow package and replace 'main' with NodeConnectionType.Main

Juneezee · 2025-01-07T13:51:53Z

To fix the errors you can use the NodeConnectionType enum from the n8n-workflow package and replace 'main' with NodeConnectionType.Main

@tomi Thanks for your reply. This fixes the TypeScript error, but the ESLint error still remain. The ESLint rule from this plugin https://github.com/ivov/eslint-plugin-n8n-nodes-base needs to be updated. See

Use NodeConnectionType consistently across code base

c33f4fc

RicardoE105 changed the title ~~Use NodeConnectionType consistently across code base~~ refactor: Use NodeConnectionType consistently across code base (no-changelog) Aug 28, 2024

RicardoE105 changed the title ~~refactor: Use NodeConnectionType consistently across code base (no-changelog)~~ refactor: Use NodeConnectionType consistently across code base (no-changelog) Aug 28, 2024

RicardoE105 requested a review from tomi August 28, 2024 17:08

ignore type error momentarily

7004662

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team node/improvement New feature or request ui Enhancement in /editor-ui or /design-system labels Aug 28, 2024

RicardoE105 added 2 commits August 28, 2024 13:39

more types fixes

9d4cf35

more types fixes

b73a5f5

RicardoE105 changed the title ~~refactor: Use NodeConnectionType consistently across code base (no-changelog)~~ refactor: Use NodeConnectionType consistently the across code base (no-changelog) Aug 28, 2024

linting fixes

d9ea284

RicardoE105 commented Aug 28, 2024

View reviewed changes

fix linting issues in node-base

045512d

RicardoE105 force-pushed the refactor-types-NodeConnectionType branch from 6e9da19 to 045512d Compare August 28, 2024 18:17

RicardoE105 changed the title ~~refactor: Use NodeConnectionType consistently the across code base (no-changelog)~~ refactor: Use NodeConnectionType consistentlt across the code base (no-changelog) Aug 28, 2024

RicardoE105 changed the title ~~refactor: Use NodeConnectionType consistentlt across the code base (no-changelog)~~ refactor: Use NodeConnectionType consistently across the code base (no-changelog) Aug 28, 2024

RicardoE105 mentioned this pull request Aug 28, 2024

refactor(editor): Unify ConnectionType and NodeConnectionTypes (no-changelog) #10562

Closed

4 tasks

fix more linting issue in editor-ui

8cf7eec

tomi reviewed Aug 29, 2024

View reviewed changes

RicardoE105 added 5 commits August 29, 2024 08:17

revert changes in nodes typing when getting credentials

e9c81b1

remove ts ignore

5c92476

revert changes to package-lock.yml

289b752

merge master

59c8a45

fix linting issue in HttpRequestV3

93a6afa

RicardoE105 requested a review from tomi August 29, 2024 13:20

tomi approved these changes Aug 29, 2024

View reviewed changes

RicardoE105 merged commit c4eb374 into master Aug 29, 2024
41 of 51 checks passed

RicardoE105 deleted the refactor-types-NodeConnectionType branch August 29, 2024 13:55

janober added the Released label Sep 5, 2024

Juneezee reviewed Jan 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Use `NodeConnectionType` consistently across the code base (no-changelog) #10595

refactor: Use `NodeConnectionType` consistently across the code base (no-changelog) #10595

RicardoE105 commented Aug 28, 2024 •

edited

Loading

cypress bot commented Aug 28, 2024 •

edited

Loading

RicardoE105 Aug 28, 2024 •

edited

Loading

tomi Aug 29, 2024

RicardoE105 Aug 29, 2024

tomi left a comment

tomi Aug 29, 2024

tomi Aug 29, 2024

RicardoE105 Aug 29, 2024

RicardoE105 Aug 29, 2024

tomi Aug 29, 2024

tomi left a comment

github-actions bot commented Aug 29, 2024

janober commented Sep 5, 2024

anantanandgupta commented Sep 12, 2024

Juneezee left a comment

tomi commented Jan 7, 2025

Juneezee commented Jan 7, 2025

refactor: Use NodeConnectionType consistently across the code base (no-changelog) #10595

refactor: Use NodeConnectionType consistently across the code base (no-changelog) #10595

Conversation

RicardoE105 commented Aug 28, 2024 • edited Loading

Summary

Review / Merge checklist

cypress bot commented Aug 28, 2024 • edited Loading

RicardoE105 Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomi left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 29, 2024

janober commented Sep 5, 2024

anantanandgupta commented Sep 12, 2024

Juneezee left a comment

Choose a reason for hiding this comment

tomi commented Jan 7, 2025

Juneezee commented Jan 7, 2025

refactor: Use `NodeConnectionType` consistently across the code base (no-changelog) #10595

refactor: Use `NodeConnectionType` consistently across the code base (no-changelog) #10595

RicardoE105 commented Aug 28, 2024 •

edited

Loading

cypress bot commented Aug 28, 2024 •

edited

Loading

RicardoE105 Aug 28, 2024 •

edited

Loading