Diagnostic for undeclared external dependencies does not respect includeNodeModules

[aminya]

🐛 bug report

#6564 breaks Parcel for Atom. atom is a built-in dependency that is not declared inside dependencies. It is implied because all the code runs inside the Atom environment.

Previously, we could specify such dependencies in includeNodeModules and set it to false, but this PR seems to disable that capability

❯ pnpm build

> atomic-terminal@1.1.5 build C:\@atom-ide-community\terminal
> cross-env NODE_ENV=production parcel build --target main ./src/terminal.ts --no-scope-hoist

× Build failed.

@parcel/core: Failed to resolve 'atom' from './src/terminal.ts'

  C:\@atom-ide-community\terminal\src\terminal.ts:1:82
  > 1 | import { CompositeDisposable, Workspace, Dock, Pane, WorkspaceOpenOptions } from "atom"
  >   |                                                                                  ^^^^^^
    2 |
    3 | import { createTerminalElement } from "./element"

@parcel/resolver-default: External dependency "atom" is not declared in package.json.

  C:\@atom-ide-community\terminal\package.json:56:3
    55 |   },
  > 56 |   "dependencies": {
  >    |   ^^^^^^^^^^^^^^
    57 |     "fs-extra": "^10.0.0",
    58 |     "node-pty-prebuilt-multiarch": "^0.10.0",

  ℹ Add "atom" as a dependency.
 ERROR  Command failed with exit code 1.

🎛 Configuration (.babelrc, package.json, cli command)

Here is the target:

  "targets": {
    "main": {
      "context": "electron-renderer",
      "engines": {
        "electron": ">=6.x"
      },
      "includeNodeModules": {
        "atom": false,
        "electron": false,
        "node-pty-prebuilt-multiarch": false
      },
      "isLibrary": true
    }
  }

🤔 Expected Behavior

The parcel should not care about the dependencies that are set as false in includeNodeModules

😯 Current Behavior

It errors.

💁 Possible Solution

Downgrade Parcel

🔦 Context

💻 Code Sample

https://github.com/atom-community/terminal

🌍 Your Environment

Software	Version(s)
Parcel	2.0.0-nightly.767
Node	all
npm/Yarn	all
Operating System	all

[devongovett]

Interesting. The diagnostic was intended to catch issues just like this though, e.g. you're publishing a library and want something to be an external. Generally when someone consumes that code it will break unless you have a dependency declared. I think in this case Parcel simply doesn't have enough information to know that atom is a builtin dependency. Maybe we need a way to configure that case specifically?

I think this would be a problem for other environments as well, like electron. We currently handle builtins by default for node, but for other environments we don't. Maybe it should be some kind of alias configuration? Aliasing a module to false currently results in an empty module, so it would need to be something different. Maybe something like:

{
  "alias": {
    "atom": { "builtin": "atom" }
  }
}

[aminya]

I think adding an envDependencies option would be nicer.

{
  "envDependencies": {
    "atom": "^1.55",
	"electron": "^6.0.0"
  }
}

[mischnic]

There's a similar situation with aws-sdk, which I believe is also injected/mocked at runtime by the AWS runtime: #4547 (comment)

[devongovett]

I think adding an envDependencies option would be nicer.

What about using the engines field for this? I see in your package.json an atom field there already, and I think for electron you'd specify that as well.

[aminya]

Yes, that works.

[ptitjes]

I believe #6603 is not enough.

For instances:

• electron-is-dev requires electron but don't declare electron in neither engines, dependencies, nor peerDependencies: https://github.com/sindresorhus/electron-is-dev/blob/main/package.json
• electron-unhandled requires electron but don't declare electron in neither engines, dependencies, nor peerDependencies: https://github.com/sindresorhus/electron-unhandled/blob/main/package.json

What can I do to fix this without dropping these dependencies ?

[aminya]

1. Make a PR to that dependency to update their engines in the package.json

2. I haven't tested it, but pnpm allows adding information that is missing from package.json of your dependencies. So, this might work:

{
  "pnpm": {
    "packageExtensions": {
      "electron-is-dev": {
        "engines": {
          "electron": "*"
        }
      },
      "electron-unhandled": {
        "engines": {
          "electron": "*"
        }
      }
    }
  }
}

[devongovett]

We do also have an electron environment in parcel, so we should probably mark the electron module as a builtin anyway.