Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

fix: Error for rootDir only when necessary #29

Merged

Conversation

stephan-noel
Copy link
Contributor

@stephan-noel stephan-noel commented Dec 24, 2020

Description

While the checking for rootDir has surfaced bugs in other locations, it is causing problems in certain cases.

!fsInstance.existsSync(rootDir) is an incorrect condition and is checking cases it shouldn't be for the following reasons:

  • The main purpose of this error was to prevent globby from throwing a vague error message and to fail fast with a clear error message. Globby does not use our fsInstance however, it always uses fs. Therefore we only need to check when we will call globby and we must do so on fs, not fsInstance.
  • When fsInstance is Volume (memfs) fsInstance.existsSync(rootDir) will fail if rootDir is aWindows path even if that path actually exists on the physical system.

This PR makes it so that we only check when we will invoke globby, and we do so on fs, which globby uses anyway, not fsInstance.

Fixes #30

How Has This Been Tested?

In addition to updating the unit tests, I reproduced it locally in a monorepo using garment and made sure the error did not happen with the fix.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have added tests to cover my changes
  • I have made corresponding changes to the documentation

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

@stephan-noel stephan-noel force-pushed the fix/glob-warning-in-minimal-cases branch from b4d5eaf to bb86038 Compare December 28, 2020 11:28
@stephan-noel stephan-noel force-pushed the fix/glob-warning-in-minimal-cases branch from bb86038 to 080462a Compare December 30, 2020 16:02
@stephan-noel stephan-noel changed the title WIP: fix: Error for rootDir only when necessary fix: Error for rootDir only when necessary Dec 30, 2020
@beshanoe beshanoe merged commit a16a853 into Farfetch:master Dec 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning about glob occurs at wrong time part 2 and createFileInput for Volume
2 participants