-
Notifications
You must be signed in to change notification settings - Fork 104
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
feat: add support for jsconfig.json #199
Conversation
@jonaskello please help review this pr. thanks |
@jonaskello do you have time to review this feature? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be more efficient to use readdirSync
here, which is only one I/O call instead of two existsSync
I/O calls.
@F3n67u I've asked @aleclarson to review this one |
@aleclarson thanks for your time. I already use |
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
} | ||
|
||
const parentDirectory = path.join(directory, "../"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonaskello Was there a good reason for using path.join
instead of path.dirname
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path.join('/root/dir', '../')
return/root/
path.dirname('/root/dir')
return/root
(without the trailing directory separators)
the latter is normalized, easier to mock when we mock the fs.readdirSync
, so I adjust path.join
to path.dirname
; though path.join(directory, '..')
is fine either.
Co-authored-by: Alec Larson <1925840+aleclarson@users.noreply.github.com>
@aleclarson could help take a look at other 2 pr I made? |
@aleclarson I see you approved this pr. do you have enough permission to merge this pr? |
@F3n67u Can you also update the README and CHANGELOG (under unreleased entry)? After that we should be good to merge. |
all done! thanks for your time. @jonaskello |
@aleclarson @dividab @jonaskello Any idea when this will be released? |
@jonaskello Would it be possible to temporarily revert #205 and cut a release already please? This would help the work I'm doing on Remix Stacks (see remix-run/indie-stack#63, remix-run/blues-stack#57 & remix-run/grunge-stack#50) a lot. |
@MichaelDeBoey This is released now in 4.0.0. |
@jonaskello Are there any breaking changes? |
The breaking changes are listed under the "changed" heading in the changelog. |
This has a bug. We should be setting |
close #127