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

Allow to require from internal project file with import/no-extraneous-dependencies #2066

Closed
piranna opened this issue May 14, 2021 · 13 comments · Fixed by #2097
Closed

Allow to require from internal project file with import/no-extraneous-dependencies #2066

piranna opened this issue May 14, 2021 · 13 comments · Fixed by #2097

Comments

@piranna
Copy link

piranna commented May 14, 2021

When importing or requireing an internal file instead of the raw project export, import/no-extraneous-dependencies fails. This is a valid use case, so for example requires to @apollo/client/link/error should be successful if the @apollo/client package is included in package.json file, but right now it's failing since it checks for the full string.

@ljharb ljharb added the bug label May 14, 2021
@ljharb
Copy link
Member

ljharb commented May 14, 2021

May be related to #2065 (cc @paztis)

@piranna
Copy link
Author

piranna commented May 14, 2021

Seems related, yes.

@paztis
Copy link

paztis commented May 17, 2021

As I explain in the defect #2072, we're recursively searching for the 1st package.json in the @apollo/client/link/error path.
Here we found it in @appolo/client folder
It means a dependency to @appolo/client need to be added.
What exactly is the error you have ? and what is the resolver you use ?

@piranna
Copy link
Author

piranna commented May 17, 2021

The error I have is that's asking to install a package that its name is the full @apollo/client/link/error, instead of just only @apollo/client. I'm using the default resolver used by Node.js v16.

@paztis
Copy link

paztis commented May 17, 2021

Is there any package.json under the folder node_modules/@apollo/client/link/error ?

@piranna
Copy link
Author

piranna commented May 17, 2021

Is there any package.json under the folder node_modules/@apollo/client/link/error ?

Surprissingly yes, I would bet it was not there before:

{
  "name": "@apollo/client/link/error",
  "main": "error.cjs.js",
  "module": "index.js",
  "types": "index.d.ts",
  "sideEffects": false
}

It doesn't make any sense, I've reviewed other folders in the installed @apollo/client package and there's other package.json files, I don't understand what does they do there...

@piranna
Copy link
Author

piranna commented May 17, 2021

@paztis
Copy link

paztis commented May 17, 2021

Try a fresh install then verify the folder.

@paztis
Copy link

paztis commented May 17, 2021

solution I propose in #2072 might resolve your problem. It's will be a little less strict on the imports but might be enough for these cases

@gabrielrtakeda
Copy link

I think that my problem is related to this...

I'm getting the linting error:

Screen Shot 2021-05-24 at 11 34 04

Where I'm importing like:

image

In my node_modules I have:

image

And have preact in package.json dependencies:

Screen Shot 2021-05-24 at 11 52 19

Is this actually, the expected behavior?
I start to get this linting error after v2.23.0

@paztis
Copy link

paztis commented May 24, 2021

Ok understood. Preact/hook is a real package with a package.json.
But this package is. Only available under master preact one.

I think one fix can solve it and all other defects.
The module resolution returns us 'preact/hooks'
We can validate it we've exactly this on or one of its ancestors in our dependencies.

If we found package a/b/c, no errors if in dependencies we've got 'a' or 'a/b' or 'a/b/c'

@ljharb what do you think about ?

@ljharb
Copy link
Member

ljharb commented May 24, 2021

preact/hooks doesn't start with @ or contain node_modules, so yes, that should come from the preact package.

@paztis
Copy link

paztis commented May 25, 2021

I've created the PR #2097 for this. Can you look at it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants