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

Circular dependency index.js ➜ parsing.js ➜ index.js #43

Open
WarningImHack3r opened this issue Apr 20, 2024 · 4 comments
Open

Circular dependency index.js ➜ parsing.js ➜ index.js #43

WarningImHack3r opened this issue Apr 20, 2024 · 4 comments

Comments

@WarningImHack3r
Copy link

Describe the bug

When building my Svelte app that uses deepl-node, I get a warning about circular dependencies:

Circular dependency: node_modules/.pnpm/deepl-node@1.13.0/node_modules/deepl-node/dist/index.js -> node_modules/.pnpm/deepl-node@1.13.0/node_modules/deepl-node/dist/parsing.js -> node_modules/.pnpm/deepl-node@1.13.0/node_modules/deepl-node/dist/index.js

To Reproduce
Steps to reproduce the behavior:

  1. Create a (Svelte) app
  2. Install, import (and use?) deepl-node in your code
  3. Build it

Expected behavior
The warning not to appear by refactoring your code

Desktop (please complete the following information):

  • OS: macOS
  • Browser: Arc
  • Version: 1.39.0
@JanEbbing
Copy link
Member

Hi, could you provide more detailed steps how to reproduce this? I just did the following (with node v18.18.2 and deepl-node v1.13.0) in a new directory:

  • npm create svelte@latest test-app
  • cd test-app
  • npm add deepl-node
  • npm install
  • Edit src/app.d.ts to look like the following
import * as deepl from 'deepl-node';


// See https://kit.svelte.dev/docs/types#app
// for information about these interfaces
declare global {
	namespace App {
		// interface Error {}
		// interface Locals {}
		// interface PageData {}
		// interface PageState {}
		// interface Platform {}
	}
}

async function translate() {
	let t = deepl.Translator('auth_key');
	let result = await t.translateText('sample text', null, 'de');
	return result;
}

export {};
  • npm run dev

And I dont get this warning.

@WarningImHack3r
Copy link
Author

Hi, I don't have access to my code right now, but it didn't show up on a dev server, only on a full prod build (npm run build).
It's pretty easy to spot though, the index.js file imports stuff from parsing.js, and parsing.js also imports stuff from index.js, thus creating a circular import.
It's not a big problem as you only import specific functions from one another but it's just a "bad practice", hence the warning

@JanEbbing
Copy link
Member

Ah sorry, I didn't get that it's just a warning. I agree, we should move a bunch of definitions into types.ts. I'll take a look.

@WarningImHack3r
Copy link
Author

Sorry about the wording, thanks!

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

No branches or pull requests

2 participants