Prefer parent directory over index for module name

[pshrmn]

This is purely an aesthetic request, but one that I think would make reading the output of Rollup much easier.

Currently, when this plugin is determining a module's name [0], it uses the basename of the id [1] to determine the module's name. If the id is /node_modules/SomePackage/module.js, we end up with the module name module. If the id was /node_modules/SomePackage/index.js, then we end up with the module name index.

The difference between these two is that for the former, we have to explicitly state that we want to import from that file, while for the latter, we only have to provide the package, since index.js is the default file to resolve to.

import X from 'SomePackage/module';
import Y from 'SomePackage';

Anecdotally, I do the latter a lot more, which means that I end up with a lot of index (and deconflicted index$1, etc.) variables throughout the bundle. Functionally this is fine, but it makes it difficult to follow when actually reading through the bundle's code and Ctrl+f for index has a few dozen results.

What this PR does is to check the output of getName prior to returning, and if it is index (and the user has specified to avoid index names), then the name of the parent segment from the directory path is used instead.

// validVar/index.js
module.exports = 'one';

// currently outputs
var index = 'one'

export default index;
export { index as _moduleExports };

// and with this PR becomes
var validVar = 'one'

export default validVar;
export { validVar as _moduleExports };

This also takes into account directory names that are invalid as variable names.

// invalid-var/index.js
module.exports = 'one';

// becomes
var invalidVar = 'one'

export default invalidVar;
export { invalidVar as _moduleExports };

Currently, this PR doesn't have any tests. I took a look at the current tests and TBH I wasn't exactly sure how I should be testing this. If this is something that you decide to move forward with, I'll obviously add tests, but I could use some guidance on their setup.

[pshrmn]

I forgot to mention that this would be non-default behavior, which only would be applied when the user specifies to do this in the options.

commonjs({
  avoidIndexName: true
})

[Rich-Harris]

Thanks for the PR @pshrmn — this is a great idea, and I actually think it should be default behaviour rather than opt-in, since there's really no disadvantage to it that I can think of. Any chance you could amend the PR to remove the option?

It would need a test, yeah — easiest way is probably to adapt an existing test and use an assertion like this to verify that it's not using index as a name. But no worries if you run into difficulties, I can always add a test later.

[pshrmn]

I removed the option and added a test. There are three cases that it verifies:

1. When the file's name is not index.
2. When the file's name is index and its parent directory's name is a valid variable name.
3. When the file's name is index and its parent directory's name is not a valid variable name.

[Rich-Harris]

thank you! released as 8.2.0