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

First pass at config file #11686

Closed
ry opened this issue Aug 13, 2021 · 5 comments
Closed

First pass at config file #11686

ry opened this issue Aug 13, 2021 · 5 comments
Assignees
Labels
feat new feature (which has been agreed to/accepted)

Comments

@ry
Copy link
Member

ry commented Aug 13, 2021

Part of #3179

  1. Deno will lookup a file called Deno.json in the current working directory on startup.
  2. This Deno.json can also be specified by the already existing --config flag
  3. Deno.json can have a top-level "compilerOptions" for configuring typescript.
  4. We will initially support one other top-level configuration called "lint" which will allow people to configure how deno lint works. Example:
{
  "compilerOptions": {
    "jsxFactory": "h"
  },

  "lint": {
    "ignore": ["testdata/"],
    "rules": {
      "tags": ["recommended"],
      "include": [
        "ban-untagged-todo"
      ]
    }
  }
}
@ry ry added the feat new feature (which has been agreed to/accepted) label Aug 13, 2021
@bartlomieju bartlomieju self-assigned this Aug 13, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Aug 13, 2021

Just to comment that I have no objections to this.

The auto discovery is going to be slightly problematic for the language server, especially with multi root workspaces like vscode supports.

@bartlomieju
Copy link
Member

Here's an an example config file from dlint (the example binary produced in deno_lint repo):

{
    "rules": {
        "tags": ["recommended"],
        "include": [
            "ban-untagged-todo"
        ],
        "exclude": [
            "no-explicit-any"
        ]
    },
    "files": {
        "include": [
            "benchmarks/oak/**/*.ts"
        ],
        "exclude": []
    }
}

It's heavily based on what dprint does, ie. it has "files" section that allows to specify "include" and "exclude" configuration. It also supports globbing. We'd need to alter the schema a bit to conform to current flags configuration. My question is should we actually add support for globs (it's been highly requested feature, eg. #6365).

Additionally if we are to add config for fmt we might reuse a bulk of existing logic from dprint. @dsherret any thoughts?

@SpadeAceman
Copy link

  1. Deno will lookup a file called Deno.json in the current working directory on startup.

Using the current working directory doesn't sound right to me. I'd expect it to look for Deno.json in the same directory as the JavaScript/TypeScript file I'm running/formatting/linting, which won't always be the current working directory.

  1. This Deno.json can also be specified by the already existing --config flag

Just to be clear, in future will the --config flag override/prevent automatic Deno.json loading?

Also, in future we'll need a way to tell Deno not to load any config files at all – either via something like --config=none, or a new dedicated command line flag (say, --no-config). I'm basing this conclusion on the fact that both ESLint and PowerShell have command-line options overriding their own automatic config-loading behavior (--no-eslintrc and -NoProfile, respectively). I'm sure there are other examples like this I'm not familiar with.

Personally, I think it's better to always require the user to specify a configuration file with the --config flag. This avoids any "spooky action at a distance" problems (and subsequent troubleshooting & documentation complexity) that can come with automatic config loading.

Nevertheless, I can understand the appeal of loading Deno.json automatically. It's a convenient way to ensure that all files in a given directory will consistently use the options you want. It also provides a standardized config file name for everyone to work with (especially if a subcommand like deno init is implemented to create it).

If you haven't already, I encourage you to take a broader look at how other projects have handled the pros and cons of automatic config loading (beyond just the JS ecosystem), to arrive at a well-considered solution with the right set of limits and trade-offs for Deno.

@bartlomieju
Copy link
Member

With #11776 and #11944 landed, this issue can be now closed.

@sant123
Copy link

sant123 commented Sep 16, 2021

I agree with @kitsonk and @SpadeAceman for the problematics it may introduce. Just want to recall one design principle of Deno no magical resolution:

https://deno.land/[email protected]/typescript/types#types-and-type-declarations

That should be adopted everywhere in the code.

Just saying 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

No branches or pull requests

5 participants