Skip to content
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: declare missing dependencies on csv-stringify and fs-extra #2326

Merged
merged 1 commit into from Feb 9, 2024

Conversation

redabacha
Copy link
Contributor

a quick win where this is the only thing that prevents crawlee from running via yarn pnp out of the box. the basic-crawler package was using two dependencies not specified in its package.json (csv-stringify and fs-extra) as also mentioned by running yarn dlx @yarnpkg/doctor ./packages/basic:

➤ YN0000: ┌ /home/reda/Documents/Projects/forks/crawlee/packages/basic-crawler/package.json
➤ YN0000: │ /home/reda/Documents/Projects/forks/crawlee/packages/basic-crawler/src/internals/basic-crawler.ts:54:1: Undeclared dependency on csv-stringify
➤ YN0000: │ /home/reda/Documents/Projects/forks/crawlee/packages/basic-crawler/src/internals/basic-crawler.ts:55:1: Undeclared dependency on fs-extra
➤ YN0000: │ /home/reda/Documents/Projects/forks/crawlee/packages/basic-crawler/src/internals/basic-crawler.ts:1441:93: Strings should avoid referencing the node_modules directory (prefer require.resolve)

there's the no-extraneous-dependencies rule from eslint-plugin-import that helps to address these kinds of problems but i can see it's been explicitly disabled in the main .eslintrc.json file so i haven't opted to re-enable it here.

@B4nan
Copy link
Member

B4nan commented Feb 9, 2024

Thanks!

there's the no-extraneous-dependencies rule from eslint-plugin-import that helps to address these kinds of problems but i can see it's been explicitly disabled in the main .eslintrc.json file so i haven't opted to re-enable it here.

I think it had a huge perf cost so I disabled it, but maybe I am confusing it with some other rule. Will check and reenable if it won't cause significant slowdown.

@B4nan B4nan changed the title fix(basic-crawler): add missing dependencies fix: declare missing dependencies on csv-stringify and fs-extra Feb 9, 2024
@B4nan B4nan merged commit 718959d into apify:master Feb 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants