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

Should the keys to tsconfig be case insensitive #1933

Closed
basarat opened this issue Feb 5, 2015 · 8 comments
Closed

Should the keys to tsconfig be case insensitive #1933

basarat opened this issue Feb 5, 2015 · 8 comments

Comments

@basarat
Copy link
Contributor

basarat commented Feb 5, 2015

Would the following be a valid tsconfig.json file:

{
    "compilerOptions": {
        "target": "es5",
        "module": "commonjs",
        "declaration": false,
        "noimplicitany": false,
        "removecomments": true
    }
}

Notice the casing on noimplicitany and removecomments.

basarat referenced this issue in TypeStrong/atom-typescript Feb 5, 2015
@csnover
Copy link
Contributor

csnover commented Feb 5, 2015

Respecfully, -1 on this being valid. :) JSON files are always case-sensitive, just like the rest of JavaScript. There’s no reason why users shouldn’t be capable of following basic conventions.

@DanielRosenwasser
Copy link
Member

I personally feel like this should be valid, since the user experience is probably better as a result of it and we're already case insensitive on the command line.

@csnover
Copy link
Contributor

csnover commented Feb 5, 2015

Command lines are case insensitive for various historical reasons; it just adds overhead and confusion to make JSON files case-insensitive, since they never are anywhere else. I guarantee you will end up with users that have noImplicitAny in one place and noimplicitany a few lines down, a linter won’t catch this, and they will be confused why the value isn’t what they think it should be.

@csnover
Copy link
Contributor

csnover commented Feb 5, 2015

I should correct my previous statement. Command line arguments in Windows are usually case-insensitive (though with powershell maybe this is not the case any more) because of their DOS heritage. In *nix, they are all case sensitive. TypeScript’s cli is an aberration in the *nix world. See for example ls -a versus ls -A. They do two different things. If you run git log --STAT it will throw an error “unrecognized argument: --STAT”.

@RyanCavanaugh
Copy link
Member

We could probably reasonably say that noImplicitAny and noimplicitany are valid, but not e.g. noIMPLICitAnY. I know working with node's readFile vs readdir vs existsSync vs symlink is really annoying.

@csnover
Copy link
Contributor

csnover commented Feb 5, 2015

We could probably reasonably say that noImplicitAny and noimplicitany are valid, but not e.g. noIMPLICitAnY

I don’t understand why this would be better.

It is not idiomatic in JavaScript or JSON to treat properties as case-insensitive. The design goals of TypeScript talk a lot about being idiomatic; doing this would not be. So I think there would need to be a significant benefit to actually doing it, right? But actually the parser already throws very clear, descriptive errors if an argument is unknown, and that feedback is provided to users immediately. It’s not like you’re burying a typo in some deep code branch that is only executed once in a blue moon. If someone typos a property they’re going to know about it the next time they compile.

I know working with node's readFile vs readdir vs existsSync vs symlink is really annoying.

The main reason why Node’s file APIs are annoying as you’ve describe them is because they are inconsistent, not because they are case-sensitive. They copied some names from libc (readdir, symlink) and made up some of their own (readFile, existsSync). They even failed to follow their own callback-pattern in at least one case (exists). As long as TypeScript is consistent—and it is—this really is a non-issue for authors. :)

@basarat
Copy link
Contributor Author

basarat commented Feb 5, 2015

We've decided to go full case sensitive same as the current compiler tsconfig implementation.

My objective was primarily to bring to notice in case the implementation changes.

@basarat
Copy link
Contributor Author

basarat commented Feb 24, 2015

go full case sensitive same as the current compiler tsconfig implementation

So I'll close this. Staying this way is fine by me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants