search

sindresorhus / import-local

Let a globally installed package use a locally installed version of itself if available
MIT License
154 stars 11 forks source link

Fix regression for projects using Lerna #5

Closed gjbkz closed 5 years ago

gjbkz commented 5 years ago

I wrote this PR because ava@2.2.0 exits with 0 without running a test in my project uses Lerna.

This is the sample code in the README:

https://github.com/sindresorhus/import-local/blob/3b6d85bea895f8ef3acad55e7fb7d06a4d77a2aa/readme.md#L18-L24

And this is the current (v3.0.1) index.js:

https://github.com/sindresorhus/import-local/blob/3b6d85bea895f8ef3acad55e7fb7d06a4d77a2aa/index.js#L6-L15

If filename === localFile, the function returns truthy value now (v3.0.1), and the process never executes the else-statements. AVA CLI uses the same code hence it causes false negatives in CI.

Here's a reproduction of this issue:

v2

v3

I reverted a condition and comments from v2.0.0.

gjbkz commented 5 years ago

I have no idea why CI fails. I've found the causes and a workaround.

Here's a CI results of my first commit (2025319) that just changes .travis-ci.yml:
https://travis-ci.com/kei-ito/import-local/builds/118569647

sindresorhus commented 5 years ago

// @mxmul

gjbkz commented 5 years ago

I added a test for my patch and found the causes of CI failures. They are

  1. "ava": "^2.1.0" at package.json#L40 and
  2. package-lock=false at .npmrc#L1 and
  3. the existence of ava@2.2.0.

After ava@2.2.0 was published on July 09, 2019, npm install installs ava@2.2.0 which depends on import-local@3.0.1.

$ npm ls import-local
import-local@3.0.1 /Users/kei.ito/workspace/import-local
└─┬ ava@2.2.0
  └── import-local@3.0.1

Then, the global test executes import-local-fixture command installed by the import-local@3.0.1:

// fixtures/cli.js
#!/usr/bin/env node
'use strict';
const importLocal = require('..');

if (importLocal(__filename)) { // → Returns truthy value
  console.log('local');        // → This is called unexpectedly
}

The failed CI jobs reproduced the issue described at top.

There are 2 ways to fix:

  1. Fixing the version number of AVA (CI Results)
"devDependencies": {
-    "ava": "^2.1.0"
+    "ava": "2.1.0"

This is first aid. When updating AVA later, make sure that it depends on import-local including this patch.

I've verified that this patch fixes the CI failures.

  1. Import self as devDependency (CI Results)
"devDependencies": {
    "ava": "^2.1.0",
+   "import-local": "file:",

This make sure that require('import-local') returns itself in this project. This looks good but the test failed on Windows.

shellscape commented 5 years ago

@mxmul Please put some eyes on this one. It's a rather large regression.