ModusCreateOrg / gimbal

Web Performance Auditing tooling
https://labs.moduscreate.com/gimbal-web-performance-audit-budgeting
MIT License
115 stars 8 forks source link

size report: no output for custom build dir #145

Closed tpraxl closed 4 years ago

tpraxl commented 4 years ago

Describe the bug The size report behaves as if build buildDir was hardcoded:

You can always see: Size Checks [ success: ✓ ], but the actual files are missing if you're not working in build.

Use Case Directories Config Expected Actual Works
1 public dir present
build dir not present
buildDir: public You can see public/index.html You can't see: public/index.html x
2 public dir present
build dir present
buildDir: public You can see public/index.html You can see: build/index.html x
3 public dir not present
build dir present
buildDir: build You can see build/index.html You can see: build/index.html

To Reproduce Steps to reproduce the behavior:

  1. Create a public dir with an index.html
  2. Create a .gimbalrc.yml with buildDir: public
  3. Run gimbal
  4. No file listed at all in the size report
  5. Copy the public dir to build
  6. Leave your config untouched (buildDir: public)
  7. Run gimbal
  8. build/index.html is listed instead of public/index.html
  9. Change your config to buildDir: build
  10. Run gimbal
  11. build/index.html is listed as expected

I have attached a bash script. For each use case, it sets up the folders and runs gimbal, so that you can see the effect and inspect the files before proceeding to the next use case. Consider using it. (Extract and chmod u+x setup-proof-of-bug.sh. It will create a minimum "project" in the folder build-dir-bug).

Expected behavior

Files are always listed for the configured buildDir. (I.e.: buildDir: public -> public/index.html is listed). Only if no buildDir is configured, it defaults to build.

Desktop (please complete the following information):

Additional context

$ uname -a
Linux joseph 4.15.0-58-generic #64~16.04.1-Ubuntu SMP Wed Aug 7 14:10:35 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
$ node --version       
v10.16.3
$ npm --version           
6.9.0
$ gimbal --version                                               
      ___                       ___                         ___
     /\__\                     /\  \         _____         /\  \
    /:/ _/_       ___         |::\  \       /::\  \       /::\  \
   /:/ /\  \     /\__\        |:|:\  \     /:/\:\  \     /:/\:\  \
  /:/ /::\  \   /:/__/      __|:|\:\  \   /:/ /::\__\   /:/ /::\  \   ___     ___
 /:/__\/\:\__\ /::\  \     /::::|_\:\__\ /:/_/:/\:|__| /:/_/:/\:\__\ /\  \   /\__\
 \:\  \ /:/  / \/\:\  \__  \:\~~\  \/__/ \:\/:/ /:/  / \:\/:/  \/__/ \:\  \ /:/  /
  \:\  /:/  /     \:\/\__\  \:\  \        \::/_/:/  /   \::/__/       \:\  /:/  /
   \:\/:/  /       \::/  /   \:\  \        \:\/:/  /     \:\  \        \:\/:/  /
    \::/  /        /:/  /     \:\__\        \::/  /       \:\__\        \::/  /
     \/__/         \/__/       \/__/         \/__/         \/__/         \/__/

  _                 __  __               _                    ____                         _
 | |__    _   _    |  \/  |   ___     __| |  _   _   ___     / ___|  _ __    ___    __ _  | |_    ___
 | '_ \  | | | |   | |\/| |  / _ \   / _` | | | | | / __|   | |     | '__|  / _ \  / _` | | __|  / _ \
 | |_) | | |_| |   | |  | | | (_) | | (_| | | |_| | \__ \   | |___  | |    |  __/ | (_| | | |_  |  __/
 |_.__/   \__, |   |_|  |_|  \___/   \__,_|  \__,_| |___/    \____| |_|     \___|  \__,_|  \__|  \___|
          |___/
 ─────────────────────────────────────────────────────────────────────────────────────────────────────

1.1.14
tpraxl commented 4 years ago

Found it. It's not a bug:

First and foremost: The size module uses configs.size.threshold settings for the folders to inspect. The configs.buildDir seems to be irrelevant for it.

I had a typo (treshold instead of threshold) in my real life project and no configs.size.threshold setting at all in my reduced "project" tests and in the examples to reproduce the issue.

That made the size module fall back to the default values (child folders of build).

So I made the mistake. But it was extremely hard to spot. I needed to debug the code to find that using trial and error.

Makes me wonder if you should add a --debug flag that prints out things like (using default values for config.size.threshold) or (you have unrecognized config values in your yml: …). Not sure though, if this would be worth the effort.

Anyways. At least, the size module should yell when there are configured folders that don't exist.

grgur commented 4 years ago

Thank you for reporting this @tpraxl. I can see opportunities for improving the docs, too. Contributions welcome (hint, hint 😄)

mitchellsimoens commented 4 years ago

FYI, part of #146 was to fix this by changing the default config to use the buildDir. I'll likely release this in a new minor version instead of just a patch but there are some other things that need to be prepped.

tpraxl commented 4 years ago

@mitchellsimoens thanks for the update