Sitelet https://github.com/import-js/eslint-plugin-import/pull/1860
Skip to content

[New]: add no-relative-packages - #1860

Merged
ljharb merged 1 commit into
import-js:masterfrom
tapayne88:feature/no-relative-packages
Jan 31, 2021
Merged

ljharb merged 1 commit into
import-js:masterfrom
tapayne88:feature/no-relative-packages

Conversation

@tapayne88

Copy link
Copy Markdown
Contributor

Duplicating #966 and hopefully fixed it 🤞 (sorry if this isn't the process).

Use this rule to prevent importing packages through relative paths.

It's useful in Yarn/Lerna workspaces, were it's possible to import a sibling package using `../package` relative path, while direct `package` is the correct one.

Co-authored-by: Rafal Lindemann <rl@stamina.pl>
Co-authored-by: Tom Payne <tom@tompayne.dev>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
@coveralls

coveralls commented Jul 23, 2020 •

Copy link
Copy Markdown

Coverage Status

Coverage increased (+5.3%) to 75.918% when pulling 6f5c52c on tapayne88:feature/no-relative-packages into 319d0ca on benmosher:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 97.837% when pulling 0aec1d3 on tapayne88:feature/no-relative-packages into 3e65a70 on benmosher:master.

@tapayne88
tapayne88 force-pushed the feature/no-relative-packages branch 6 times, most recently from 3467f06 to 300a5b3 Compare July 23, 2020 22:14
@tapayne88

Copy link
Copy Markdown
Contributor Author

@ljharb is there anything I can do to get this over the line?

@tapayne88
tapayne88 force-pushed the feature/no-relative-packages branch from 300a5b3 to 273a9e9 Compare September 17, 2020 08:19
@wycats

wycats commented Dec 3, 2020 •

Copy link
Copy Markdown

@tapayne88 I tried this out from your branch and it improved my workflow dramatically.

One recommendation: right now, the warning suggests importing from package-name/index instead of package-name. It seems like that ought not happen.

One wrinkle: in my case, I am not importing from the same location as the main entry point (which is dist/index.js) because the mistaken import is accidentally the index.ts in the root of the package. I have types pointing at index.ts, but that is probably not a perfect heuristic. In a situation where the import is pointing at an index.ts file that isn't the main entry point, perhaps it's just fine to suggest package-name?

@tapayne88

Copy link
Copy Markdown
Contributor Author

hey @wycats, thanks for the feedback! To be honest, the bulk of the work was originally done by @panrafal but I'll try set aside some time to look into your recommendation - it does sound closer to what we want.

If I'm understanding your wrinkle correctly, I think node module resolution should use the "main" field in the package.json first (if available) and resort to the index.js/index.ts second before heading up the filesystem in search of more node_modules folders so I think what you're seeing is the desired behaviour.

@wycats

wycats commented Dec 9, 2020

Copy link
Copy Markdown

@tapayne88 If it finds index.ts in the absence of a types field (even in the presence of main field), I think that would be ok.

@wycats

wycats commented Dec 9, 2020

Copy link
Copy Markdown

@ljharb what do you think of this PR?

@ljharb ljharb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Went ahead and rebased, and refactored to use the new moduleVisitor pattern.

@ljharb
ljharb force-pushed the feature/no-relative-packages branch from e6533d2 to 6f5c52c Compare January 31, 2021 17:05
@ljharb
ljharb merged commit 6f5c52c into import-js:master Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants