Skip to content

n8n-nodes-apify improve code quality #21

@protoss70

Description

@protoss70

Improve the n8n-nodes-apify repo code according to the code reviews done by n8n team on this ticket

You can check out the following two tickets which have the same issues implemented to the n8n-nodes-website-content-crawler app
PR 1
PR 2

How

All of these issues are solved in the n8n-nodes-apify-content-crawler repository. So if you ever need inspiration please check it out. The two repos are mostly similar anyways.

Here is a list of issues we need to fix in our n8n app:

1. genericFunctions.ts

Replace the deprecated function with httpRequestWithAuthentication

46:69 error 'requestWithAuthentication' is deprecated. Use 'httpRequestWithAuthentication' instead for better authentication support and consistency @n8n/community-nodes/no-deprecated-workflow-functions

2. Apify.node.ts

  • Item pairing: We need to add item pairing and continueOnFail while calling the execute function. Example below is from n8n WCC repo which is functioning.
async execute(this: IExecuteFunctions): Promise<INodeExecutionData[][]> {
  const items = this.getInputData();
  const returnData: INodeExecutionData[] = [];

  for (let i = 0; i < items.length; i++) {
  	try {
  		const data = await actorsRouter.call(this, i);

  		const addPairedItem = (item: INodeExecutionData) => ({
  			...item,
  			pairedItem: { item: i },
  		});

  		if (Array.isArray(data)) {
  			returnData.push(...data.map(addPairedItem));
  		} else {
  			returnData.push(addPairedItem(data));
  		}
  	} catch (error) {
  		if (this.continueOnFail()) {
  			returnData.push({
  				json: { error: error.message },
  				pairedItem: { item: i },
  			});
  			continue;
  		}
  		throw error;
  	}
  }

  return [returnData];
}

3. package.json and testing

  • We need to remove the n8n-core package and move the n8n-workflow package to the peerDependencies. This will cause our tests to fail which will be the most annoying part of this ticket. Mock every function necessary until the tests run again.

  • Remove the preinstall script.

4. Code quality issue

Code Quality Issues
Finding: Inconsistent error handling pattern genericFunctions.ts:60
Uses error.constructor?.name === 'NodeApiError' instead of instanceof

5. Replace pnpm with npm

We need to start using npm instead of pnpm. This will require some changes to the CI pipelines as well.

Validating fixes

Metadata

Metadata

Assignees

Labels

debtCode quality improvement or decrease of technical debt.medium priorityMedium priority issues to be done in a couple of sprints.t-integrationsIssues with this label are in the ownership of the integrations team.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions