Move from untyped->typed config

[lemonmade]

I'm working through adding a linting command for Sewing Kit, and it's much easier if the "base" config of the project has no type-checking commands; it lets us automatically create a "typed" version for CI (this doesn't work well right now because the editor config needs to extend the "full" config, then disable type checking, which means the "full" config always needs to exist).

[GoodForOneFare]

Is the suggestion that:

1. There will be less friction when bootstrapping a project because only one config is necessary
2. CI will automatically do a fully-typed lint, even if the user has not defined a script to run tslint with --type-check

?

[lemonmade]

The way I have this working is that, when you run the sewing-kit lint command, it creates a private config file that extends the project's config (which now just includes the default/ React configs) and uses the correct CLI arguments to get type checking. This means only one config file for the project, which makes things a lot simpler

[ismail-syed]

I think we'll need to change ./build/untyped to ./build/typed ./build/base in this project's tslint.json

[ismail-syed]

Finished 🎩 with Neutron. No issues 👍

[GoodForOneFare]

Please don't ship yet.

[ismail-syed]

Gord says 🙅‍♂️

[ismail-syed]

Thanks for remembering to 🎩 with Atom! ❤️

Atom's error messages a super obnoxious so it's easy to see if things break.

[ismail-syed]

The config that editors pick up shouldn't have any type checking rules enabled, we shouldn't be seeing foo-bar-rule requires type checking. 🤔

Let's sit down together tomorrow and see where this 💩 the bed.

[GoodForOneFare]

There's no 💩, AFAIC. I'm okay with VS Code displaying some warnings buried in a console.

I think the config split is an artifact of when the lint plugins gave ridiculous feedback on typed rules (popped up a warning dialog on every save, IIRC?)

---

You can easily get type warnings by adding an override. e.g.,

Create custom-rules.js:

module.exports = {
  rules: {
    "await-promise": true,
  },
};

Then adding them after the base rules:

{
  "extends": [
    "tslint-config-shopify",
    "./custom-rules.js"
  ]
}

[ismail-syed]

Was able to reproduce the Warning: The 'await-promise' rule requires type checking in the VSCode output console tab.

I'm okay with it buried in the console a well. But I'm not okay with a pop-up warning on every save. I'm not seeing any of those. @GoodForOneFare have you noticed any of those obnoxious pop-up warnings with this new setup.

[lemonmade]

Based on these discussions, I completely wiped out my changes and got rid of the multiple config stuff entirely. Can you guys take another look?

[ismail-syed]

LGTM 👍

What's the benefit of the untyped + full config nowadays? In the past, the typed config made VS Code unusable.

Were there recent changes to VSCode that allowed this work?

[ismail-syed]

Maye this? https://code.visualstudio.com/updates/v1_11#_source-control

[GoodForOneFare]

No, not that. tbh, I can't even remember which editor was problematic. I was more of an Atom user back then, so you could start by searching through the linter-tslint repo for answer.

[GoodForOneFare]

Out of paranoia, I also tested in Sublime. Displays the same one-time console errors as vscode, and works perfectly.

[GoodForOneFare]

While reviewing the neutron tslint bump, I found this post about how VS Code used to do nothing when typed rules were present:

This behavior was changed in TSLint recently (palantir/tslint#2169) so that TSLint now just prints a warning message and continues linting. That should remove one of the big issues mentioned above, so it ought to be possible to just use one tslint.json file now instead of multiple.

So that explains the convoluted nature of tslint-config-shopify@<3.