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

Linting Standardisation: Import Sort Order, Member Order, Member Accessibility, Type Imports, Spaced Comment, Usage of fs #272

Closed
4 tasks done
CMCDragonkai opened this issue Oct 26, 2021 · 13 comments
Assignees
Labels
procedure Action that must be executed

Comments

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Oct 26, 2021

Incorporate the new import sort order standard from MatrixAI/TypeScript-Demo-Lib@393bc34

This should ensure that our imports are always aligned in the same order.

This should be done after the loose ends are merged from #240 and #259 as they involve some changes that could screw them up. Instead this will affect likely all the files.

Tasks

  1. - Add in the eslint-plugin-import NPM dependency
  2. - Run the npm run lintfix
  3. - Do a full test to see if everything still works
  4. - Merge

All the rules that will be useful here:

@CMCDragonkai CMCDragonkai added the procedure Action that must be executed label Oct 26, 2021
@CMCDragonkai
Copy link
Member Author

Rule explanation is here: https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/order.md

I've added in the minimal amount of configuration that I think can work.

@CMCDragonkai
Copy link
Member Author

Missed one rule for @ itself, so it should be this instead, already updated on TS Demo Lib:

    "import/order": [
      "error",
      {
        "groups": [
          "type",
          "builtin",
          "external",
          "internal",
          "index",
          "sibling",
          "parent",
          "object"
        ],
        "pathGroups": [
          {
            "pattern": "@",
            "group": "internal"
          },
          {
            "pattern": "@/**",
            "group": "internal"
          }
        ]
      }
    ],

@CMCDragonkai
Copy link
Member Author

This can now be done. Should squeeze it in quick so that everybody's imports can be in the right order.

@joshuakarp
Copy link
Contributor

Supporting work - can be quickly added and checked once Gitlab MR is merged.

@joshuakarp joshuakarp mentioned this issue Nov 16, 2021
3 tasks
@CMCDragonkai
Copy link
Member Author

@CMCDragonkai CMCDragonkai changed the title Import Sort Order Standardisation Linting Standardisation: Import Sort Order, Member Order, Member Accessibility, Type Imports, Spaced Comment, Usage of fs Nov 21, 2021
@CMCDragonkai
Copy link
Member Author

Renamed issue to indicate all the linting standardisation we want to use.

@CMCDragonkai
Copy link
Member Author

@CMCDragonkai
Copy link
Member Author

For https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/213

  1. Import sorting is applied
  2. Spaced comment is applied
  3. No throw literal also applied
  4. Consistent type imports too

@CMCDragonkai
Copy link
Member Author

The node plugin is not needed to control imports. In fact we may need to import fs legitimately in our tests and in some places in src until we can create our own FileSystem type fully.

@CMCDragonkai
Copy link
Member Author

For access modifiers, we can do this when we update the typescript-eslint and use this:

    "@typescript-eslint/explicit-member-accessibility": ["error", {
      "accessibility": "explicit",
      "overrides": {
        "parameterProperties": "off"
      }
    }],

That will do auto fixing. But we won't add it yet.

@CMCDragonkai
Copy link
Member Author

With the merging of https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/213.

All of this is done except:

Both cannot be done for now because they are not autofixable until we upgrade our eslint dependencies.

I think this is acceptable for now so we're going to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
procedure Action that must be executed
Development

No branches or pull requests

2 participants