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

Babel cli fix recursion in file mode #8418

Merged

Conversation

bitcloud
Copy link
Contributor

@bitcloud bitcloud commented Aug 3, 2018

Q                       A
Fixed Issues?
Patch: Bug Fix? Y
Major: Breaking Change? N
Minor: New Feature? N
Tests Added + Pass? Y
Documentation PR Link
Any Dependency Changes? N
License MIT

Why

While using babel-cli in file mode and passing it a folder to recurse throu (e.g. babel src --out-file test.js) it just scans for all files in the provided directory but does not recurse into it's subdirectories.

Currently it uses an extension filter that is applied to all folder elements before processing. But the extension filter doesn't exclude directories from filtering.
This results in unexpected behaviour because it would recurse into folders that would have an extension, but not in those who don't. And concerning the documentation and its behaviour in its previous version, it should recurse into all directories.

How

The PR adds an additional check to the recurse filter and accepts all directories.

@babel-bot
Copy link
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8757/

@babel babel deleted a comment from babel-bot Aug 3, 2018
Copy link
Member

@loganfsmyth loganfsmyth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@loganfsmyth loganfsmyth merged commit cb51704 into babel:master Aug 3, 2018
@TrySound
Copy link
Contributor

TrySound commented Aug 7, 2018

@bitcloud You should've upgrade fs-readdir-recursive package. 1.0.0 doesn't have api you use in this PR. Because of this change after updating from 55 to 56 beta we get the error like this
#8431

hzoo pushed a commit that referenced this pull request Aug 8, 2018
PR #8418 introduced some new logic that uses the third parameter of `fs-readdir-recursive`'s `filter` option, the current directory, but missed that our dependencies only required `^1.0.0`, while that parameter was actually added in `1.1.0`. This means that if some other dependency forced a downgrade in the version to satisfy some range, or the user already had 1.0.0 module installed locally, it could cause problems,
@bitcloud
Copy link
Contributor Author

bitcloud commented Aug 8, 2018

ah, damn, sorry about that. Was working with my current freshly installed env so didn't catch that one. Is there actually a way to install all minimal versions of required packages to test something like that?

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants