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

Update: Added --extension/-x parameter overwrite default .js extension #35

Merged
merged 1 commit into from
Jan 30, 2017

Conversation

Guibod
Copy link
Contributor

@Guibod Guibod commented Jan 29, 2017

As seen in #6 it is important to allow people to safely choose what extension they want to support.

I've created this PR to allow people to create-index over jsx and js, but it can be extended to anything.
To achieve that readDirectory accepts extensions in its options. If not specified, it is defaulted to 'js'.

The main change is that file extension is tested against all of those extension to see if the file is eligible for export.
Second important feature is that removeDuplicates also search for files that are homonyms, and will always discard homonyms with extensions matching the first listed. So if you put extensions = ['js', 'jsx'] you'll always let your js file win over jsx files. It works as for homonyms directories.

@gajus
Copy link
Owner

gajus commented Jan 29, 2017

Whats the reason anyone would want to use a different extension for "jsx" files?

@Guibod
Copy link
Contributor Author

Guibod commented Jan 29, 2017

I really don't know. :)
My point was to allow any extensions, but to enable them to properly configure them, through an explicit option. For instance, you may want to import json, or css as JS object. I don't know.

The point is that I need to generate an index for directories mixing JS and JSX, past this point I don't know what would be the usage. I tried to be as open as possible in this implementation.

If you want to enforce only js and jsx extension, you only need to hide the -x option from command-line.

@gajus
Copy link
Owner

gajus commented Jan 29, 2017

My problem is that it allows (and therefore promotes) code style that has no merits.

@Guibod
Copy link
Contributor Author

Guibod commented Jan 30, 2017

So, to be clear, you want to hardcode a list of allowed extensions. I guess that's limited to JS and JSX, right ? How do we handle homonyms, is .js file always wins over .jsx ?

@gajus
Copy link
Owner

gajus commented Jan 30, 2017

So, to be clear, you want to hardcode a list of allowed extensions. I guess that's limited to JS and JSX, right ? How do we handle homonyms, is .js file always wins over .jsx ?

No. I am suggesting not to allow any other extensions that are non-.js.

@Guibod
Copy link
Contributor Author

Guibod commented Jan 30, 2017

In #6 people are in dire need of this feature. And it is a common pattern to use the jsx extension.
Well enough, your house your rules. I have nothing to add, please close if you don't want it.

@gajus
Copy link
Owner

gajus commented Jan 30, 2017

Well enough, your house your rules. I have nothing to add, please close if you don't want it.

I appreciate this attitude. :-)

I the #6 issue I said I will accept the PR. Therefore, it wouldn't be fair to just close this.

Thank you for contributing.

@gajus gajus merged commit 5fea5e6 into gajus:master Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants