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 docs incorrectly imply that Babel supports generic blob pattern matching in ignore/only and such #9680

Closed
jcollum-nike opened this issue Mar 14, 2019 · 11 comments
Labels
i: bug i: needs triage outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@jcollum-nike
Copy link

jcollum-nike commented Mar 14, 2019

Bug Report

Current Behavior
The file in src/__mocks__/@company is processed but Glob will ignore that file when given the same pattern.

Input Code
https://github.com/jcollum-nike/babel-glob-bug

Run node glob-test.js:

[ 'src/index.js' ]
[ 'src/index.js' ]
[ 'src/index.js' ]

Then run these commands:

# nr = npm run (alias) 
nr --silent clean && nr --silent build:pass1 && tree cjs
Successfully compiled 1 file with Babel.
cjs
└── index.js

nr --silent clean && nr --silent build:pass2 && tree cjs
Successfully compiled 1 file with Babel.
cjs
└── index.js

nr --silent clean && nr --silent build:fail1 && tree cjs 
Successfully compiled 2 files with Babel.
cjs
├── __mocks__
│   └── @company
│       └── some-fn.js
└── index.js

Expected behavior/code
The file in src/__mocks__/@company is ignored in all three scenarios when processed with babel CLI.

Environment

  • Babel version(s): 7.2.3 (@babel/core 7.3.4)
  • Node/npm version: Node 8.15
  • OS: OSX
  • Monorepo: no
  • How you are using Babel: cli in package.json
@babel-bot
Copy link
Collaborator

Hey @jcollum-nike! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@loganfsmyth
Copy link
Member

This is expected behavior, Babel has extremely minimal support for globs: https://babeljs.io/docs/en/options#matchpattern

We literally only global-match when * and ** are whole-path-segment placeholders, so __* in this case would not be treated as a glob, but rather as a folder with that specific name.

If you want to use more complex glob matching, you can always use a JS config file and pass a function that uses the glob library used in your example. Then you can toggle that with --env-name or your own environment variable or whatever you'd like.

@jcollum-nike
Copy link
Author

jcollum-nike commented Mar 14, 2019

Please reopen because your docs don't say what you are saying here. The docs:

npx babel src --out-dir lib --ignore "src/**/*.spec.js","src/**/*.test.js"

That sure implies that babel cli uses normal globs. https://babeljs.io/docs/en/babel-cli#ignore-files

Also I feel like you didn't actually look at the package.json in the repo provided

@loganfsmyth
Copy link
Member

Fair enough, those docs appear to be outdated.

Also I feel like you didn't actually look at the package.json in the repo provided

My previous comment quotes __* right from your example failure of --ignore "**/__*/**" so that seems like an unfair characterization.

@loganfsmyth loganfsmyth reopened this Mar 14, 2019
@loganfsmyth loganfsmyth changed the title Babel is processing file in src/__mocks__/@company that is ignored by Glob when given the same pattern Babel-cli docs incorrectly imply that Babel supports generic blob pattern matching in ignore/only and such Mar 14, 2019
@jcollum-nike
Copy link
Author

Well the point is that it's not failing in the glob test -- that's why it's called fail1

You have glob as a dependency, aren't you just passing those ignore patterns into the glob lib directly? That seems like the simplest way to do it.

@loganfsmyth
Copy link
Member

loganfsmyth commented Mar 14, 2019

Well the point is that it's not failing in the glob test -- that's why it's called fail1

Your glob test demonstrates that your ignore using **/__*/** does not ignore src/__mocks__/@company/some-fn.js. That implies that the glob failed to match and thus failed to ignore it.

You have glob as a dependency, aren't you just passing those ignore patterns into the glob lib directly?

Looks like it is presently used only for the filenames passed to the CLI, not for anything else.

That seems like the simplest way to do it.

We explicitly dropped general glob usage because we want to treat ignore and such as file paths, e.g. ignore: [path.join(__dirname, "test")] and the glob module explicitly expects all arguments to be forward-slash separated meaning that we could either allow ignore to work on Windows, or we could use glob, but not both. We chose to keep the API path-focused.

@jcollum-nike
Copy link
Author

Your glob test demonstrates that your ignore using /__*/ does not ignore src/mocks/@company/some-fn.js.

This is incorrect. The glob test shows that all three globs ignore the same files. The initial bug report has the output of the glob test.

@jcollum-nike
Copy link
Author

jcollum-nike commented Mar 14, 2019

meaning that we could either allow ignore to work on Windows, or we could use glob, but not both. We chose to keep the API path-focused.

The glob docs don't say anything about it not working on Windows, only that it will interpret /foo to be c:\foo\, which seems reasonable.

@loganfsmyth
Copy link
Member

This is incorrect. The glob test shows that all three globs ignore the same files. The initial bug report has the output of the glob test.

I think we're referring to different examples. I'm referring to the examples of Babel running with your --ignore globs, not your glob-test.js script, e.g.

nr --silent clean && nr --silent build:pass2 && tree cjs
Successfully compiled 1 file with Babel.
cjs
└── index.js

nr --silent clean && nr --silent build:fail1 && tree cjs 
Successfully compiled 2 files with Babel.
cjs
├── __mocks__
│   └── @company
│       └── some-fn.js
└── index.js

where both of them don't ignore the same files, or else the output would be the same.

The glob docs don't say anything about it not working on Windows, only that it will interpret /foo to be c:\foo, which seems reasonable.

The docs there state

You must use forward-slashes only in glob expressions. Back-slashes will always be interpreted as escape characters, not path separators.

which means that if a user did --ignore C:\foo\bar\index.js would treat the backslashes as an escape character instead of path separators, so it would fail to ignore that file.

@jcollum-nike
Copy link
Author

jcollum-nike commented Mar 14, 2019

where both of them don't ignore the same files, or else the output would be the same.

And that's why it's a bug: of the 3 patterns, all three behave the same way in Glob but only 2 of 3 behave the same way when passed to the Babel CLI. So it's either a bug or the docs need some indication that the globs in Babel don't work the same way as the globs in Glob.

Because, considering that Glob is a dependency of this tool it seems very surprising (Principle of Least Surprise after all) that the glob patterns would behave differently in the Babel CLI.

@loganfsmyth
Copy link
Member

So it's either a bug or the docs need some indication that the globs in Babel don't work the same way as the globs in Glob.

I agree that the docs need to be updated. I've filed babel/website#1986 since the website docs are not tracked in this repo.

Because, considering that Glob is a dependency of this tool it seems very surprising (Principle of Least Surprise after all) that the glob patterns would behave differently in the Babel CLI.

The dependencies of a package really isn't something to depend on when considering functionality, specially since Babel is split across many packages. Making assumptions based on that can lead to confusion exactly like what you're raising here. ignore is a feature of Babel as a whole, not the CLI, so it is implemented @babel/core which has no dependency on glob.

Gonna close since the doc update is tracked in the other repo, but happy to continue discussing.

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

No branches or pull requests

3 participants