rework handling and docs for invalid installations (#2089)

* use is-installed-globally, emit warning

* disable lint rule

* remove unused dependency

* rough implementation of runtime check

* update comment

* include method name you called

* tweak wording of runtime error

* tweak global message

* expand documentation

* update link to docs

* tweak wording again

* rework testing a bit

* tweak variable naming

* update CHANGELOG.md

* address review comments

* fix typo

Co-authored-by: Aurélien Reeves <[email protected]>

Co-authored-by: Aurélien Reeves <[email protected]>

CHANGELOG.md

@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 Please see [CONTRIBUTING.md](https://github.com/cucumber/cucumber/blob/master/CONTRIBUTING.md) on how to contribute to Cucumber.
 
 ## [Unreleased]
+### Changed
+- Reworked handling for invalid installations ([#2089](https://github.com/cucumber/cucumber-js/pull/2089))
 
 ## [8.4.0] - 2022-06-29
 ### Fixed

README.md

@@ -87,6 +87,7 @@ If you learn best by example, we have [a repo with several example projects](htt
 
 The following documentation is for `main`, which might contain some unreleased features. See [documentation for older versions](./docs/older_versions.md) if you need it.
 
+* [Installation](./docs/installation.md)
 * [CLI](./docs/cli.md)
 * [Configuration](./docs/configuration.md)
 * Support Code

docs/cli.md

@@ -22,9 +22,6 @@ Or via [npx](https://docs.npmjs.com/cli/v8/commands/npx):
 $ npx cucumber-js
 ```
 
-**Note on global installs:** Cucumber does not work when installed globally because `@cucumber/cucumber`
-needs to be required in your support files and globally installed modules cannot be required.
-
 ## Options
 
 All the [standard configuration options](./configuration.md#options) can be provided via the CLI.

docs/faq.md

@@ -17,25 +17,4 @@ If you have similar `Given` and `Then` patterns, try adding the word “should
 
 ## Why am I seeing `The "from" argument must be of type string. Received type undefined`?
 
-If when running cucumber-js you see an error with a stack trace like:
-
-```
-TypeError [ERR_INVALID_ARG_TYPE]: The "from" argument must be of type string. Received type undefined
-    at validateString (internal/validators.js:125:11)
-    at Object.relative (path.js:1162:5)
-    ...
-```
-
-This usually an effect of one of:
-
-- Your project depends on cucumber-js, and also has a dependency (in `node_modules`) that depends on cucumber-js at a different version
-- You have a package that depends (even as a dev dependency) on cucumber-js linked (via `npm link` or `yarn link`)
-
-These cases can cause two different instances of cucumber-js to be in play at runtime, which causes errors.
-
-If removing the duplicate dependency is not possible, you can work around this by using [import-cwd](https://www.npmjs.com/package/import-cwd) so your support code always requires cucumber-js from the current working directory (i.e. your host project):
-
-```js
-const importCwd = require('import-cwd')
-const { Given, When, Then } = importCwd('@cucumber/cucumber')
-```
+See [Invalid installations](./installation.md#invalid-installations)

docs/installation.md

@@ -0,0 +1,76 @@
+# Installation
+
+With [npm](https://www.npmjs.com/):
+
+```shell
+$ npm install @cucumber/cucumber
+```
+
+With [Yarn](https://yarnpkg.com/):
+
+```shell
+$ yarn add @cucumber/cucumber
+```
+
+## Invalid installations
+
+If Cucumber exits with an error message like:
+
+```
+You're calling functions (e.g. "Given") on an instance of Cucumber that isn't running.
+This means you have an invalid installation, mostly likely due to:
+...
+```
+
+This means you have an invalid installation.
+
+Unlike many libraries, Cucumber is _stateful_; you call functions to register your support code, and we keep that state until it's used in the test run. Therefore, it's important that everything interacting with Cucumber in your project is interacting with the same instance. There are a few ways this can go wrong:
+
+### Global installation
+
+Some libraries with a command-line interface are designed to be installed globally. Not Cucumber though - for the reasons above, you need to install it as a dependency in your project.
+
+We'll emit a warning if it looks like Cucumber is installed globally.
+
+### Duplicate dependency
+
+If your project depends on `@cucumber/cucumber`, but also has another dependency that _itself_ depends on `@cucumber/cucumber` (maybe at a slightly different version), this can cause the issue with multiple instances in play at the same time. If you're familiar with React, this is a lot like [the "invalid hook call" issue](https://reactjs.org/warnings/invalid-hook-call-warning.html#duplicate-react).
+
+This is common where you have split some of your support code (e.g. step definitions) into a separate package for reuse across multiple projects, or are perhaps using a third-party package intended to work with Cucumber.
+
+You can diagnose this by running `npm why @cucumber/cucumber` in your project. You might see something like:
+
+```
+@cucumber/[email protected] dev
+node_modules/@cucumber/cucumber
+  dev @cucumber/cucumber@"8.4.0" from the root project
+
+@cucumber/[email protected] dev
+node_modules/my-shared-steps-library/node_modules/@cucumber/cucumber
+  dev @cucumber/cucumber@"8.3.0" from [email protected]
+  node_modules/my-shared-steps-library
+    my-shared-steps-library@"1.0.0" from the root project
+```
+
+In this case, the fix is to change the library so `@cucumber/cucumber` is a [peer dependency](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#peerdependencies) rather than a regular dependency (it probably also needs to be a dev dependency). This will remove the duplication in the host project. If you don't control the library, consider using [overrides](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#overrides) (npm) or [resolutions](https://classic.yarnpkg.com/lang/en/docs/selective-version-resolutions/) (Yarn) to get it down to a single instance.
+
+### Deprecated package
+
+When looking at the duplicate dependency issue, it's worth checking whether anything in your project is depending on the old, deprecated `cucumber` package. Anything touching Cucumber [should be using](../UPGRADING.md#package-name) the newer `@cucumber/cucumber` package.
+
+### Linking
+
+With the shared library example above, even if you have `@cucumber/cucumber` correctly defined as a peer dependency, you can still hit the issue if you hook up the library locally using `npm link` or `yarn link` when developing or testing.
+
+This is trickier to deal with. If you run `npm link ../my-project/node_modules/@cucumber/cucumber` from the library, this should work around it (assuming `my-project` is your host project's directory, and it's adjacent to your library in the file system).
+
+### Notes
+
+In earlier versions of Cucumber, this issue would present with a more cryptic error (the causes and solutions are the same):
+
+```
+TypeError [ERR_INVALID_ARG_TYPE]: The "from" argument must be of type string. Received type undefined
+    at validateString (internal/validators.js:125:11)
+    at Object.relative (path.js:1162:5)
+    ...
+```

features/invalid_installation.feature

@@ -0,0 +1,28 @@
+Feature: Invalid installations
+
+  @spawn
+  Scenario: Cucumber exits with an error when running an invalid installation
+    Given an invalid installation
+    Given a file named "features/a.feature" with:
+      """
+      Feature: some feature
+        Scenario:
+          When a step is passing
+      """
+    And a file named "features/step_definitions/cucumber_steps.js" with:
+      """
+      const {When} = require('@cucumber/cucumber')
+
+      When(/^a step is passing$/, function() {})
+      """
+    When I run cucumber-js
+    Then it fails
+    And the error output contains the text:
+      """
+      You're calling functions (e.g. "When") on an instance of Cucumber that isn't running.
+      This means you have an invalid installation, mostly likely due to:
+      - Cucumber being installed globally
+      - A project structure where your support code is depending on a different instance of Cucumber
+      Either way, you'll need to address this in order for Cucumber to work.
+      See https://github.com/cucumber/cucumber-js/blob/main/docs/installation.md#invalid-installations
+      """

features/step_definitions/cli_steps.ts

@@ -76,22 +76,6 @@ When(
   }
 )
 
-When(
-  'I run cucumber-js \\(installed locally\\)',
-  { timeout: 10000 },
-  async function (this: World) {
-    return await this.run(this.localExecutablePath, [])
-  }
-)
-
-When(
-  'I run cucumber-js \\(installed globally\\)',
-  { timeout: 10000 },
-  async function (this: World) {
-    return await this.run(this.globalExecutablePath, [])
-  }
-)
-
 Then('it passes', () => {}) // eslint-disable-line @typescript-eslint/no-empty-function
 
 Then('it fails', function (this: World) {

features/step_definitions/install_steps.ts

@@ -0,0 +1,56 @@
+import { Given } from '../../'
+import tmp from 'tmp'
+import path from 'path'
+import fs from 'fs'
+import fsExtra from 'fs-extra'
+import { World } from '../support/world'
+
+/*
+Simulates something like a global install, where the Cucumber being executed
+is not the one being imported by support code
+ */
+Given('an invalid installation', async function (this: World) {
+  const projectPath = path.join(__dirname, '..', '..')
+  const tmpObject = tmp.dirSync({ unsafeCleanup: true })
+
+  // Symlink everything in node_modules so the fake installation has all the dependencies it needs
+  const projectNodeModulesPath = path.join(projectPath, 'node_modules')
+  const projectNodeModulesDirs = fs.readdirSync(projectNodeModulesPath)
+  const installationNodeModulesPath = path.join(tmpObject.name, 'node_modules')
+  projectNodeModulesDirs.forEach((nodeModuleDir) => {
+    let pathsToLink = [nodeModuleDir]
+    if (nodeModuleDir[0] === '@') {
+      const scopeNodeModuleDirs = fs.readdirSync(
+        path.join(projectNodeModulesPath, nodeModuleDir)
+      )
+      pathsToLink = scopeNodeModuleDirs.map((x) => path.join(nodeModuleDir, x))
+    }
+    pathsToLink.forEach((pathToLink) => {
+      const installationPackagePath = path.join(
+        installationNodeModulesPath,
+        pathToLink
+      )
+      const projectPackagePath = path.join(projectNodeModulesPath, pathToLink)
+      fsExtra.ensureSymlinkSync(projectPackagePath, installationPackagePath)
+    })
+  })
+
+  const invalidInstallationCucumberPath = path.join(
+    installationNodeModulesPath,
+    '@cucumber',
+    'cucumber'
+  )
+  const itemsToCopy = ['bin', 'lib', 'package.json']
+  itemsToCopy.forEach((item) => {
+    fsExtra.copySync(
+      path.join(projectPath, item),
+      path.join(invalidInstallationCucumberPath, item)
+    )
+  })
+
+  this.localExecutablePath = path.join(
+    invalidInstallationCucumberPath,
+    'bin',
+    'cucumber.js'
+  )
+})

features/support/hooks.ts

@@ -1,8 +1,6 @@
 import { After, Before, formatterHelpers, ITestCaseHookParameter } from '../../'
-import fs from 'fs'
 import fsExtra from 'fs-extra'
 import path from 'path'
-import tmp from 'tmp'
 import { doesHaveValue } from '../../src/value_checker'
 import { World } from './world'
 import { warnUserAboutEnablingDeveloperMode } from './warn_user_about_enabling_developer_mode'
@@ -54,57 +52,6 @@ Before('@esm', function (this: World) {
   })
 })
 
-Before('@global-install', function (this: World) {
-  const tmpObject = tmp.dirSync({ unsafeCleanup: true })
-
-  // Symlink everything in node_modules so the fake global install has all the dependencies it needs
-  const projectNodeModulesPath = path.join(projectPath, 'node_modules')
-  const projectNodeModulesDirs = fs.readdirSync(projectNodeModulesPath)
-  const globalInstallNodeModulesPath = path.join(tmpObject.name, 'node_modules')
-  projectNodeModulesDirs.forEach((nodeModuleDir) => {
-    let pathsToLink = [nodeModuleDir]
-    if (nodeModuleDir[0] === '@') {
-      const scopeNodeModuleDirs = fs.readdirSync(
-        path.join(projectNodeModulesPath, nodeModuleDir)
-      )
-      pathsToLink = scopeNodeModuleDirs.map((x) => path.join(nodeModuleDir, x))
-    }
-    pathsToLink.forEach((pathToLink) => {
-      const globalInstallNodeModulePath = path.join(
-        globalInstallNodeModulesPath,
-        pathToLink
-      )
-      const projectNodeModulePath = path.join(
-        projectNodeModulesPath,
-        pathToLink
-      )
-      fsExtra.ensureSymlinkSync(
-        projectNodeModulePath,
-        globalInstallNodeModulePath
-      )
-    })
-  })
-
-  const globalInstallCucumberPath = path.join(
-    globalInstallNodeModulesPath,
-    '@cucumber',
-    'cucumber'
-  )
-  const itemsToCopy = ['bin', 'lib', 'package.json']
-  itemsToCopy.forEach((item) => {
-    fsExtra.copySync(
-      path.join(projectPath, item),
-      path.join(globalInstallCucumberPath, item)
-    )
-  })
-
-  this.globalExecutablePath = path.join(
-    globalInstallCucumberPath,
-    'bin',
-    'cucumber.js'
-  )
-})
-
 After(async function (this: World) {
   if (this.reportServer?.started) {
     await this.reportServer.stop()

features/support/world.ts

@@ -36,7 +36,6 @@ export class World {
   public lastRun: ILastRun
   public verifiedLastRunError: boolean
   public localExecutablePath: string
-  public globalExecutablePath: string
   public reportServer: FakeReportServer
 
   parseEnvString(str: string): NodeJS.ProcessEnv {