grunt-neuter icon indicating copy to clipboard operation
grunt-neuter copied to clipboard

Fail when required file is missing. Fixes #45

Open AndreasPizsa opened this issue 10 years ago • 8 comments

I've had the situation where I accidently require()d a missing file. The app built fine, but then didn't run.

It took me a while to figure out why my app wasn't working until I found out that neuter silently ignored the missing file, concatenated the remaining ones and continued with the build. (I was using an older version of neuter that didn't warn about missing files as it was only checking for !files and not !files.length, but I see that's changed now). At runtime, of course, everything from the missing file was missing in the code and caused errors.

I think the expected behaviour is the following:

  • require() means that something is required for a build
  • wildcard names can resolve to zero or more files. So zero is ok, but probably worth knowing about.
  • requiring a specific file means that file is required. If it's missing, there is something seriously wrong.

I fixed this the following way:

If glob returns zero files and require() was called with...

  • ... a wildcard character: issue a warning and continue with the build (grunt.log.warn)
  • ... with a specific file name (no wildcards): issue a warning and fail (grunt.fail.warn).

Thanks :)

Review on Reviewable

AndreasPizsa avatar Dec 29 '13 20:12 AndreasPizsa

Seems good, but I do need a test before I can merge. I have a nasty habit of breaking untested code

trek avatar Jan 01 '14 14:01 trek

Thanks, @trek . I will add and commit tests.

AndreasPizsa avatar Jan 01 '14 18:01 AndreasPizsa

I just had this same issue. Wasted a couple hours before realizing the build was broken, not my code. require('missing-file') should always break the build so you catch mistakes sooner.

What's the status on this?

joeyespo avatar May 27 '14 18:05 joeyespo

@joeyespo The PR works but doesn't come with tests, so it's not pulled in yet.

AndreasPizsa avatar May 29 '14 06:05 AndreasPizsa

Ok.

In the meantime, I just pulled in these changes locally. It found a few more silent rouge require() calls and dramatically sped up some refactoring I was doing.

This PR is definitely useful. Need help with the tests to get this in?

joeyespo avatar May 29 '14 14:05 joeyespo

@joeyespo Actually yes, this would be very helpful. I started writing tests but came across a few issues that make them a bit more challenging; I wouldn't even want to share them at this point as they're really more attempts and ideas rather than actual starting points.

So please feel free to supply tests, you're very welcome to do so! We could finally get these changes into the main branch :)

AndreasPizsa avatar May 30 '14 08:05 AndreasPizsa

Ok. Finally got around to adding some tests in #53.

joeyespo avatar Aug 30 '14 06:08 joeyespo

Thank you, @joeyespo !

AndreasPizsa avatar Aug 30 '14 06:08 AndreasPizsa