Add `extensions` option for TypeScript support and other extensions in the future #9

huan · 2016-11-21T08:29:13Z

sindresorhus · 2016-11-21T09:20:08Z

There are more places here where .js is hardcoded. Also see my comment here: #4 (comment)

We also need to decide whether extension should replace .js or be an addition.

huan · 2016-11-21T11:52:40Z

Ah, got it.

I just did a check, and list the following parts need to add extensions support too:

About whether extensions should replace .js or be an addition, my idea is replace. That's because I believe extensions should include .js in it, and without hardcoded .js, the code can be cleaner for support extensions.

Which one do you prefer, replace or additional?

sindresorhus · 2016-11-21T15:03:56Z

Thinking about it more, replacing makes the most sense here. Why would you even use .js if you have a TypeScript project, and if you have that edge-case, it's still possible to manually specify it.

huan · 2016-11-21T15:21:45Z

Ok, so both of us agree to use replace.

I'll try to modify the code later, get rid of all part of the hard coded .js, and start using extensions to rewrite current related codes.

jednano · 2016-11-25T05:27:32Z

index.js


 				if (process.platform === 'win32') {
 					// Always use / in patterns, harmonizing matching across platforms.
-					pattern = slash(pattern);
+					// pattern = slash(pattern);
+					patternList = patternList.map(function (p) {


This should be the same as patternList.map(slash); no?

yes, you are right, it's the same. thanks!

vadimdemedes · 2016-11-27T17:40:19Z

index.js

-		'test.js',
-		'test-*.js',
+function defaultIncludePatterns(extensions) {
+	var patternList = [


Imo it could just be patterns.

vadimdemedes · 2016-11-27T17:42:22Z

index.js

@@ -241,14 +266,20 @@ function handlePaths(files, excludePatterns, globOptions) {

 				searchedParents[file] = true;

-				var pattern = path.join(file, '**', '*.js');
+				// var pattern = path.join(file, '**', '*.js');
+				var patternList = [];


patterns too?

vadimdemedes · 2016-11-27T17:43:12Z

index.js

+				// var pattern = path.join(file, '**', '*.js');
+				var patternList = [];
+				globOptions.extensions.forEach(function (ext) {
+					patternList.push(path.join(file, '**', '*.' + ext));


Inconsistency between https://github.com/avajs/ava-files/pull/9/files#diff-168726dbe96b3ce427e7fedce31bb0bcR187. path.join() is used here, unlike in that other place. I think it should be used in both places.

Although, since we are using slash(), path.join() is not necessary.

vadimdemedes · 2016-11-27T17:44:13Z

index.js

@@ -269,6 +300,16 @@ function handlePaths(files, excludePatterns, globOptions) {
 		});
 }

+function validFileName(file, extensions) {


isValidFileName would be better.

vadimdemedes · 2016-11-27T17:44:51Z

test.js

+
+	const files = await avaFiles.findTestFiles();
+	const expected = [
+		'typescript.ts'


Can be on the same line.

vadimdemedes · 2016-11-27T17:45:01Z

test.js

+	const files = await avaFiles.findTestFiles();
+	const expected = [
+		'typescript.ts'
+	].map(function (file) {


Should be an arrow function.

huan · 2016-11-28T01:17:43Z

Hey @vdemedes ,

Thanks for the review, I had followed your suggestions and made a new push.

sindresorhus · 2016-11-29T06:53:37Z

I would be more confident in this change if it had more tests.

sindresorhus · 2016-11-29T06:58:37Z

There's lot of commented code left in there. Please always review the diff before pushing changes.

huan · 2016-11-29T13:47:54Z

Sorry for the commented code because I use them to compare the code at first, and forgot to clean them. Just cleaned them all.

About the tests, I added a new test with empty extensions option, which should get nothing from ava-files.

What kind of tests do you think we need to add more? I can follow you, but now I can only think about test extensions parameter, which could be a ['ts'] or [].

novemberborn · 2017-01-17T17:44:26Z

@zixia this might prove quite useful for RFC001! However we've decided to move this code back into AVA core. I'm closing this PR since it won't land here, but I'll make sure to remember it for when we need this functionality.

huan added 5 commits October 14, 2016 20:47

create new AvaFile param: exts to set file extention

e4c164d

add unit test for new param exts

679b489

add new fixtr for new param exts

cf6191d

make XO happy

0500bb9

use extensions as param name. avajs/ava#1109

378fe1b

huan mentioned this pull request Nov 21, 2016

Add support for TypeScript without precompiling avajs/ava#1122

Closed

sindresorhus mentioned this pull request Nov 22, 2016

Finds Files With Custom Extensions #4

Closed

sindresorhus changed the title ~~Add new parameter: extensions for TypeScript Support(and other extension names in the future)~~ Addextensions option for TypeScript support and other extensions in the future Nov 22, 2016

sindresorhus changed the title ~~Addextensions option for TypeScript support and other extensions in the future~~ Add extensions option for TypeScript support and other extensions in the future Nov 22, 2016

follow #9 (comment)

a41e10e

jednano reviewed Nov 25, 2016

View reviewed changes

https://github.com/avajs/ava-files/pull/9#pullrequestreview-10111839

ab6fe45

vadimdemedes reviewed Nov 27, 2016

View reviewed changes

follow reviews from @vdemedes

a7954c6

huan added 2 commits November 29, 2016 13:41

get rid of useless comment of old codes

8e6b2d7

add test with empty extentions set

285f62b

novemberborn closed this Jan 17, 2017

huan mentioned this pull request May 4, 2017

Allow tests files to have any extension (i.e. .jsx, .ts) avajs/ava#631

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `extensions` option for TypeScript support and other extensions in the future #9

Add `extensions` option for TypeScript support and other extensions in the future #9

huan commented Nov 21, 2016

sindresorhus commented Nov 21, 2016

huan commented Nov 21, 2016 •

edited

Loading

sindresorhus commented Nov 21, 2016

huan commented Nov 21, 2016

jednano Nov 25, 2016

huan Nov 26, 2016

vadimdemedes Nov 27, 2016

huan Nov 28, 2016

vadimdemedes Nov 27, 2016

huan Nov 28, 2016

vadimdemedes Nov 27, 2016

vadimdemedes Nov 27, 2016

huan Nov 28, 2016

vadimdemedes Nov 27, 2016

huan Nov 28, 2016

vadimdemedes Nov 27, 2016

huan Nov 28, 2016

vadimdemedes Nov 27, 2016

huan Nov 28, 2016

huan commented Nov 28, 2016

sindresorhus commented Nov 29, 2016

sindresorhus commented Nov 29, 2016

huan commented Nov 29, 2016

novemberborn commented Jan 17, 2017

Add extensions option for TypeScript support and other extensions in the future #9

Add extensions option for TypeScript support and other extensions in the future #9

Conversation

huan commented Nov 21, 2016

sindresorhus commented Nov 21, 2016

huan commented Nov 21, 2016 • edited Loading

sindresorhus commented Nov 21, 2016

huan commented Nov 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huan commented Nov 28, 2016

sindresorhus commented Nov 29, 2016

sindresorhus commented Nov 29, 2016

huan commented Nov 29, 2016

novemberborn commented Jan 17, 2017

Add `extensions` option for TypeScript support and other extensions in the future #9

Add `extensions` option for TypeScript support and other extensions in the future #9

huan commented Nov 21, 2016 •

edited

Loading