Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Enhancement: Make no-relative-imports rule stricter #624

Closed
IllusionMH opened this issue Nov 1, 2018 · 3 comments
Closed

Enhancement: Make no-relative-imports rule stricter #624

IllusionMH opened this issue Nov 1, 2018 · 3 comments
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Breaking Change Type: Rule Feature Adding a feature to an existing rule.
Milestone

Comments

@IllusionMH
Copy link
Contributor

IllusionMH commented Nov 1, 2018

At this moment no-relative-imports dissalows .. only at the beginning of the import path (see #508).

Since next release most likely will be major release it is good opportunity to make "breaking" change disallow .. inside path.

This will be easy check and won't require a lot of effort.

Ideally - disallow not only /../ but also /./ inside of path.

Update rule instead of new one - because it will be basically copy-paste with minor changes in check function (that shouldn't conflict with existing) and error message.

@JoshuaKGoldberg
Copy link

Sure, if this gets in within the next couple of days let's fast-track it into 6.0. On the other hand, since no-relative-imports is an odd and rarely used rule (I've been recommending against it for some time now), if this gets in as a 6.1 change instead of 6.0.1, that feels fine too.

@JoshuaKGoldberg JoshuaKGoldberg added Status: Accepting PRs Good First Issue 🙌 Howdy, neighbor! Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Type: Breaking Change Type: Rule Feature Adding a feature to an existing rule. labels Nov 2, 2018
@IllusionMH
Copy link
Contributor Author

Implementation is ready, but there are two questions:

  1. Is it critical to preserve Imported module path starts... or External module path starts... or it can be unified to Module path starts... (in any case there will be either import ... or require('...') in error message)?
  2. Is there a need to support \ as path separator? Previously only first 1-2 characters were checked and there were no difference for path separators.

@JoshuaKGoldberg
Copy link

  1. That seems fine. It'd be nice to keep it, but if it overly complicates the implementation details, agreed that it's not critical. We can always file a followup issue to re-add it.
  2. Yes, unfortunately. Some Windows developers use \ as path separators AFAIK.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Breaking Change Type: Rule Feature Adding a feature to an existing rule.
Projects
None yet
Development

No branches or pull requests

2 participants