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

pass in glob pattern, but only get report for 1 file #157

Closed
lmj0011 opened this issue Oct 7, 2017 · 10 comments
Closed

pass in glob pattern, but only get report for 1 file #157

lmj0011 opened this issue Oct 7, 2017 · 10 comments
Labels

Comments

@lmj0011
Copy link

lmj0011 commented Oct 7, 2017

Do you want to request a feature or report a bug?
report a bug

What is the current behavior?
glob patterns seem to not work with the bundlesize cli tool

If the current behavior is a bug, please provide the steps to reproduce.

pass in a glob pattern into the bundlesize command. (see screenshots for steps)

err2

err3

  • contents of src/
    err1

What is the expected behavior?

I was expecting 10 outputs, 1 for each file in the src directory

Please mention other relevant information.

  • node version
    7.10.0

  • npm version
    4.2.0

  • Operating system
    Ubuntu 16.04.3 LTS (xenial)

  • bundlesize version
    0.15.2

  • CI you are using
    none

@siddharthkp
Copy link
Owner

@lmj0011 Nice catch!

Turns out you can't get multiple files with commander (or maybe we're using it wrong)

Ref: https://github.com/siddharthkp/bundlesize/blob/master/src/config.js#L14-L28

Thanks for taking the time to report the issue, we'll fix it!

@siddharthkp
Copy link
Owner

@lmj0011 Would you like to work on this?

CONTRIBUTING.md is a good place to start. And of course, we are here to discuss the details + implementation and help you along the way!

@lmj0011
Copy link
Author

lmj0011 commented Oct 10, 2017

sure, I can take a look at it when I have time

@timdeschryver
Copy link

timdeschryver commented Oct 12, 2017

I think this might be a bash problem.
When running node index.js -f ./src/*.js on the normal windows cmd, it shows all the files.
When running on windows bash it just outputs the first file (the other files are accessible via program.args), when running node index.js -f './src/*.js' it also shows all the files.
Also when running it as a script (in package.json) it also shows all the files.

I don't know certain but maybe bash has a glob like system built in?

Would it be an option to discard the -f option and make it like bundlesize ./src/*.js?
Would look like:

if (program.args) {
  cliConfig = program.args.map(path => ({ path, maxSize: program.maxSize }))
}

@timdeschryver
Copy link

On a side note, I expected that a glob like pattern would sum up the file sizes.
For example: it would wouldn't pass when running bundlesize globPattern -s 2kb if there are 3 files that are 1kb each.

@siddharthkp
Copy link
Owner

hmm, I think we'll have to replace this with a more explicit file read.

side note: I'm not sure if sum of sizes is a useful metric, because bundles are usually loaded in parallel, you should optimise for the slowest request

@lmj0011
Copy link
Author

lmj0011 commented Oct 14, 2017

@tdeschryver is right, BASH is parsing the glob before it gets handled by the node script.

My suggestion would be to make a note of quoting the glob path on the cli
https://github.com/siddharthkp/bundlesize#cli

see: https://medium.com/@jakubsynowiec/you-should-always-quote-your-globs-in-npm-scripts-621887a2a784

@siddharthkp
Copy link
Owner

@lmj0011 updated the readme for "quotes"

@styfle
Copy link

styfle commented Jun 13, 2018

@siddharthkp I agree with @tdeschryver...
I thought the glob pattern was going to sum the size of the matched files to see if it exceeded the given size.

Is this feature implemented already or is there a feature request for this already?

@siddharthkp
Copy link
Owner

I thought the glob pattern was going to sum the size of the matched files to see if it exceeded the given size.

Nope, sum of files is not a useful metric.

Here's the explanation: #205 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants