fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) #11329

netroy · 2024-10-21T13:21:29Z

Summary

We had an inconsistency in how and when we prefixed packageName on nodes. This was causing supportedNodes referring to invalid node names.
This PR moves all packageName prefixing code to LoadNodesAndCredentials, which ensures that lazy-loading has no effect on node-names anywhere.

Extracted out of #9874.
E2E Tests
Workflow Tests

Related Linear tickets, Github issues, and Community forum posts

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)

packages/cli/src/load-nodes-and-credentials.ts

tomi

Can't say that I 100% understood everything that changed here. The code and tests look solid and if you tested that it works I'm happy. Couple questions / comments but no blockers 🚀

packages/cli/src/load-nodes-and-credentials.ts

tomi · 2024-10-21T15:03:09Z

packages/cli/src/load-nodes-and-credentials.ts

-			for (const credentialTypeName in loader.credentialTypes) {
-				this.loaded.credentials[credentialTypeName] = loader.credentialTypes[credentialTypeName];
+			for (const type in loader.credentialTypes) {
+				this.loaded.credentials[type] = loader.credentialTypes[type];


Credentials don't need to be prefixed with the packageName?

unfortunately not. I've wanted to prefix credentials since 2022, but doing so will be a breaking change.

packages/core/src/DirectoryLoader.ts

cypress · 2024-10-21T15:27:30Z

n8n Run #8273

Run Properties: Passed #8273 • 5900e1d0be: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 netroy 🗃️ e2e/*

Project	`n8n`
Branch Review	`refactor-generate-ui-types`
Run status	`Passed #8273`
Run duration	`04m 30s`
Commit	`5900e1d0be: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 netroy 🗃️ e2e/*`
Committer	`कारतोफ्फेलस्क्रिप्ट™`
View all properties for this run ↗︎

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

github-actions · 2024-10-21T15:27:32Z

✅ All Cypress E2E specs passed

tomi

🚀

github-actions · 2024-10-22T08:23:02Z

✅ All Cypress E2E specs passed

elsmr

LGTM

I only found this change in the output of known/credentials.json. AFAIK this will not have any effect because these credentials are never used directly, only extended. But maybe you know a case where this could break things.

packages/core/src/DirectoryLoader.ts

github-actions · 2024-10-22T10:20:41Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

…(no-changelog)

codecov · 2024-12-10T00:32:33Z

Codecov Report

Attention: Patch coverage is 87.83784% with 18 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
packages/cli/src/load-nodes-and-credentials.ts	52.38%	10 Missing ⚠️
packages/core/src/DirectoryLoader.ts	92.85%	5 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

elsmr

Nice! so many added tests and simpler code + 1 less script 😄

github-actions · 2024-12-10T13:28:00Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

github-actions · 2024-12-10T13:41:35Z

✅ All Cypress E2E specs passed

janober · 2024-12-11T14:25:41Z

Got released with [email protected]

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 labels Oct 21, 2024

netroy force-pushed the refactor-generate-ui-types branch from abd08ab to 14cc34f Compare October 21, 2024 14:22

netroy changed the title ~~refactor(core): Simplify metadata generation, and add tests for DirectoryLoader (no-changelog)~~ fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) Oct 21, 2024

netroy mentioned this pull request Oct 21, 2024

fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) #9874

Closed

1 task

netroy added the tests-needed This PR needs additional tests label Oct 21, 2024

netroy commented Oct 21, 2024

View reviewed changes

packages/cli/src/load-nodes-and-credentials.ts Outdated Show resolved Hide resolved

tomi previously approved these changes Oct 21, 2024

View reviewed changes

netroy dismissed tomi’s stale review via 83ae133 October 21, 2024 15:45

netroy force-pushed the refactor-generate-ui-types branch from fe0aef9 to 83ae133 Compare October 21, 2024 15:45

netroy requested a review from elsmr October 21, 2024 15:46

tomi previously approved these changes Oct 22, 2024

View reviewed changes

elsmr previously approved these changes Oct 22, 2024

View reviewed changes

packages/core/src/DirectoryLoader.ts Show resolved Hide resolved

packages/core/src/DirectoryLoader.ts Show resolved Hide resolved

netroy mentioned this pull request Oct 24, 2024

refactor: Every node class should have a distinct constructor name (no-changelog) #11403

Merged

4 tasks

netroy dismissed stale reviews from tomi and elsmr via f4919f7 December 9, 2024 17:02

netroy force-pushed the refactor-generate-ui-types branch 2 times, most recently from f4919f7 to e50f6c4 Compare December 9, 2024 17:07

fix(core): Fix supportedNodes for non-lazy loaded community packages …

5900e1d

…(no-changelog)

netroy force-pushed the refactor-generate-ui-types branch from e50f6c4 to 5900e1d Compare December 10, 2024 00:08

netroy requested a review from elsmr December 10, 2024 08:05

elsmr approved these changes Dec 10, 2024

View reviewed changes

netroy merged commit 2d36b42 into master Dec 10, 2024
65 checks passed

netroy deleted the refactor-generate-ui-types branch December 10, 2024 13:48

janober added the Released label Dec 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) #11329

fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) #11329

netroy commented Oct 21, 2024 •

edited

Loading

tomi left a comment

tomi Oct 21, 2024

netroy Oct 21, 2024

cypress bot commented Oct 21, 2024 •

edited

Loading

github-actions bot commented Oct 21, 2024

tomi left a comment

github-actions bot commented Oct 22, 2024

elsmr left a comment

github-actions bot commented Oct 22, 2024

codecov bot commented Dec 10, 2024

elsmr left a comment

github-actions bot commented Dec 10, 2024

github-actions bot commented Dec 10, 2024

janober commented Dec 11, 2024

fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) #11329

fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) #11329

Conversation

netroy commented Oct 21, 2024 • edited Loading

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

tomi left a comment

Choose a reason for hiding this comment

tomi Oct 21, 2024

Choose a reason for hiding this comment

netroy Oct 21, 2024

Choose a reason for hiding this comment

cypress bot commented Oct 21, 2024 • edited Loading

n8n Run #8273

github-actions bot commented Oct 21, 2024

tomi left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 22, 2024

elsmr left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 22, 2024

codecov bot commented Dec 10, 2024

Codecov Report

elsmr left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 10, 2024

github-actions bot commented Dec 10, 2024

janober commented Dec 11, 2024

netroy commented Oct 21, 2024 •

edited

Loading

cypress bot commented Oct 21, 2024 •

edited

Loading