refactor(is-file): Convert to TypeScript #23093
Conversation
| ? Array.isArray(relativePath) | ||
| ? relativePath.map(withDir) | ||
| : withDir(relativePath) | ||
| : null |
There was a problem hiding this comment.
I don't think this check is relevant anymore, possibly ever. getAbsolutePath is called from getFilePath, and the relativePath parameter is a value return from slash in gatsby-core-utils which is defined as returning a string. As such, I think that Array.isArray will always return false.
There was a problem hiding this comment.
Looking at some test coverage data for the original JS file suggests that if it is possible to be an array, it's at least untested.
There was a problem hiding this comment.
I think you're right. Let's try not to make any more semantic changes in TS convert PR's than we have to. Then in a followup diff refactor the code because it looks to me like this file can use a little cleanup and modernization.
chooban
force-pushed
the
ts-migrate/is-file
branch
2 times, most recently
from
5618f8e to
b005f0e
Compare
chooban
force-pushed
the
ts-migrate/is-file
branch
from
b005f0e to
d806beb
Compare
|
Rebased and updated. I put the while loop changes into their own commit. |
|
Apologies, this dropped off of my todo list to keep it updated. Remerged up-to-date after your recent changes. |
| ? Array.isArray(relativePath) | ||
| ? relativePath.map(withDir) | ||
| : withDir(relativePath) | ||
| : null |
There was a problem hiding this comment.
I think you're right. Let's try not to make any more semantic changes in TS convert PR's than we have to. Then in a followup diff refactor the code because it looks to me like this file can use a little cleanup and modernization.
|
@chooban Thank you! 💜 |
Description
Converts
src/schema/infer/is-file.jsto TypeScript.Related Issues
References #21995
Unfortunately, no tests. I would be happy to add some, even do a prequel PR in JS, if someone could point me at a similar one which creates a NodeStore.Turns out there is test coverage, just not a unit test. If I run the full test suite then the file has good coverage from somewhere!
The order of functions in the file has change as the linter was complaining about declaration order. I guess that's a new check in TS compared to JS.I redid the conversion with the functions in the same order in the hope that it would show as a diff rather than delete and add, but no such luck.
I've put a few comments in-line in the PR to outline a few issues.