[RRFC] Don't expand tabs by default

[dgchrt]

Motivation ("The Why")

Expanded tabs can be harmful to some developers and should be avoided. Currently, new packages created with npm init produce package*.json files with their tabs expanded to a couple spaces, which in turn won't respect editor configurations. This can be specially harmful to developers which rely on custom editor configurations. The whole motivation for this is having the concept of "safe defaults" in place for the broader community.

Example

Generate the new files with their tabs intact instead.

How

Current Behaviour

Tabs expanded to a couple spaces:

{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}

Desired Behaviour

Intact tabs:

{
	"name": "test",
	"version": "1.0.0",
	"description": "",
	"main": "index.js",
	"scripts": {
		"test": "echo \"Error: no test specified\" && exit 1"
	},
	"author": "",
	"license": "ISC"
}

References

Please refer to this article if you're new to the subject: https://alexandersandberg.com/tabs-for-accessibility/
This issue was previously discussed here: npm/feedback#200

[ljharb]

For a new package, if editorconfig is present, I’d expect that to be read - separately, I’d expect an npm config value i can set to tell npm i always want tabs for indentation.

[isaacs]

Adding new --indent=<string> and --eol=<string> config options would be straightforward. Whenever we edit the package.json or package-lock.json files, we preserve whatever indentation is already in the file.

[isaacs]

Parsing editor config files might be tricky, since there are a lot of editors out here, but maybe that's worth exploring as a default. If we make it a config value, you could just put it in a project .npmrc.

[dgchrt]

I believe simply not expanding tabs by default should be sufficient. Then whoever wants customisation could further expand them to something else anyways. That's precisely what tabs are intended for. The trick is, until you have them, you can treat them however you like, but when they're pre-expanded already, it takes extra effort to undo that.

[ljharb]

isn't .editorconfig just ini? i'm pretty sure every editor treats it the same. I agree that an explicit npmrc value works, but it still would be nicer if .editorconfig populated the defaults.

It would be awesome if the default was always tabs instead of spaces, but I assume that would be much too disruptive a change (and would invite way too much bikeshedding).

[dgchrt]

Personally, I think npm init will be executed early in a new project, leaving things like .editorconfig and other customisations to a later point, when it would be already too late for that. The whole motivation for this is having the concept of "safe defaults" in place for the broader community. Let me update the issue description with that, by the way.

I'm aware that any kind of "spaces vs tabs" discussion is usually shied away, because for many developers it doesn't really matter. The problem with that is, it does matter for some developers, and then the benefit of unexpanded tabs becomes clear.

[isaacs]

In case anyone is wondering, the only reason that npm has always used 2-space indentation for json files is that npm uses 2-space indentation for code internally. The only reason why npm has always used 2-space indentation for code is because Node.js did. The reason why node did is that 2-space indentation was the standard in the ruby community and in V8, and Ryan Dahl was a rubyist and C++ dev before ever touching JavaScript. So, while I 100% agree with all of the reasons why tabs are objectively superior, and used tabs in all my code prior to 2009 for all these extremely rational reasons, it just wasn't worth having that fight while in the minority on this issue. At this point, changing the default is potentially pretty disruptive, so we'd have to be prepared to withstand a lot of noise about it.

[dgchrt]

Thanks for the clarification. Even though I fail to see how this change would disrupt any workflow, I agree that it would be impossible to predict that, e.g. someone might be doing some custom post-processing of the generated package*.json files via some custom JSON parser, that was hard-coded to specifically parse 2-spaces indented files only, and that would definitely break. Very unlikely, but still possible.

That being said, I hope we don't sacrifice the greater good for the sake of how things got the way they currently are. I'd say that if this change is properly announced in an upcoming version, it should roll out just fine.

[isaacs]

I think it would be fine if we had json-indent and json-eol config items, which could default to ' ' and '\n' for now (so we add flexibility without any potential for disruption, even minor) and then change the default indent to '\t' in a future semver-major.

It's tempting to say that someone preferring spaces over tabs is irrational, ignorant/insensitive to accessibility concerns, etc. But I've found most people have their reasons, and it's best not to rock the boat within a major version release line.

[dgchrt]

It looks like a sound plan, in my opinion. And I agree, no need to judge the reasoning behind the current state to figure out ways of moving forward.

[isaacs]

Oh, one other thing on this:

someone might be doing some custom post-processing of the generated package*.json files via some custom JSON parser, that was hard-coded to specifically parse 2-spaces indented files only, and that would definitely break. Very unlikely, but still possible.

Anyone making that assumption today is likely already dealing with a brittle and error-prone situation. npm defaults to 2-space indents, but it maintains whatever you have in package.json currently when it edits it. So, if you convert the package.json and package-lock.json to tabs in a project, that's another workaround you could use today (albeit less convenient than a config, of course).

[dgchrt]

if you convert the package.json and package-lock.json to tabs in a project, that's another workaround you could use today

Yes, that's the trick I'm using today, I'm glad it works like that. For new files, I just need to run sed once and then all is fine.

[lukekarrys]

I put together an RRFC that I believe can supersede this one and wanted to confirm it covered the changes requested here: #423 (comment)

It adds a few other config items and bikesheds the names a bit, but it should cover the cases of changing indent and eol in package*.json files, without a breaking change.

[lukekarrys]

@diogoeichert Based on the latest discussion from the RFC meeting last week (#423 (comment)), I think we landed on a proposed solution that would work here. I'm going to close this issue but you can comment over there or reopen this if you have any issues with the new proposal.