Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add grouped imports by default in tslint:all #4420

Merged
merged 3 commits into from Jan 29, 2019
Merged

Add grouped imports by default in tslint:all #4420

merged 3 commits into from Jan 29, 2019

Conversation

VincentLanglet
Copy link
Contributor

Overview of change:

The tslint:all config seems to be used in order to activate all the rules and to be the strictest possible.
"grouped-imports":true is stricter than "grouped-imports":false

CHANGELOG.md entry:

[enhancement] add grouped import in the all config

@VincentLanglet
Copy link
Contributor Author

BTW, since ordered imports is used in tslint:recommended.
Should we add the grouped imports option ?

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

overall this change seems fine, but see my comment below.

no, we shouldn't change tslint:recommended, since that configuration follows semver (see configuration docs)

import { camelize } from "../src/utils";
import { IOptions } from "./../src/language/rule/rule";

Copy link
Contributor

Choose a reason for hiding this comment

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

was this newline added by the grouped-imports option? what's the logic here? it seems to separate the different levels of imports into groups in some places, but not in others (e.g. in exclusionFactory.ts, ../../ imports are in the same group as ../ imports). I haven't been following this rule closely, but that behavior is a little surprising to me and I'd like to revisit it before enabling this rule in tslint:all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. There is 3 différents levels: libraries, parents folder and same folder.

import something from "module"

import foo from "../../foo"
import bar from "../bar"

import baz from "./baz"

@adidahiya
Copy link
Contributor

ok, the documentation for "grouped-imports" is pretty light, so I'd like to see it expanded before merging this, at the very least for other TSLint contributors so they know what to expect from this rule when working on the TSLint code base.

@VincentLanglet
Copy link
Contributor Author

@adidahiya
The first time I read this rules I found it pretty clear, there is a similar rule for eslint. I can add a commit with more explanation for the rule, but what did you not understood with the existing following explanation ?

you can enforce a grouping of third-party, parent directories and the current directory.

Group source imports by "bar", "../baz", "./foo".

@VincentLanglet
Copy link
Contributor Author

Hi @adidahiya, could you be more explicit about the lack of documentation ?
I consider the actual pretty clear, and there is a fixer. I'll be happy to add something if you need it.

Thanks

@adidahiya adidahiya self-assigned this Jan 17, 2019
@adidahiya adidahiya added this to the 5.13.0 milestone Jan 17, 2019
@adidahiya
Copy link
Contributor

with #4134 merged, the docs are now in a better place. I'll merge this and resolve any conflicts on master

@adidahiya adidahiya merged commit 95d9d95 into palantir:master Jan 29, 2019
@adidahiya adidahiya mentioned this pull request Jan 29, 2019
4 tasks
ColCh added a commit to ColCh/tslint that referenced this pull request Feb 3, 2019
* master: (60 commits)
  Added tslint-brunch to the list of 3rd party tools (palantir#4251)
  Switch to tslint-plugin-prettier, clean up rule options config syntax (palantir#4488)
  Enable grouped-imports for ordered-imports rule in tslint:all config (palantir#4420)
  Ordered imports grouping (palantir#4134)
  trailing-comma: check for a closing parenthesis (palantir#4457)
  Update index.md (palantir#4473)
  [bugfix] `no-unsafe-any`: allow implicitly downcasting `any` to `unknown` (palantir#4442)
  Add v5.12.1 changelog
  Bump version to 5.12.1
  Fix quotemark avoid-template issues (palantir#4408)
  Skip linting JSON files entirely (palantir#4001)
  Fix strict-type-predicate for unknown (palantir#4444)
  restrict increment-decrement fixer while fixing the postfix unary expressions (palantir#4415)
  Mention file names in script parse failures (palantir#4397)
  Revert breaking change to tslint:recommended, update tslint:latest (palantir#4404)
  Fix quotemark avoid-template issues (palantir#4408)
  Bump tslint dev dependency to 5.12.0 (palantir#4452)
  Skip linting JSON files entirely (palantir#4001)
  Fix strict-type-predicate for unknown (palantir#4444)
  [README] Update link for Webstorm (palantir#4450)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants