Fix: load .env value also if entry target is specified with source value in package.json

[Shinyaigeek]

↪️ Pull Request

Fixes: #7515

Hi team 👋

While I was investigating this issue, I noticed the cause of this issue.

When entry file is specified with source value in package.json ( and not specified with cli build target value), entries will be determined as ["."] in https://github.com/parcel-bundler/parcel/blob/v2/packages/core/parcel/src/cli.js#L219-L221, and resolved into project root directory in https://github.com/parcel-bundler/parcel/blob/v2/packages/core/parcel/src/cli.js#L223.
These entry files are passed into getRootDir to detect the entry root directory. .env is loaded from this entry root directory. However, when the entry file is specified with source value in package.json ( and not specified with cli build target value), these entry files are resolved into the project root directory, so getRootDir returns the parent of the project root directory, so .env file cannot be loaded due to this wrong path.

I fixed this

💻 Examples

package.json

{
 ...
 "source": "src/index.tsx"
 ...
 "scripts": {
   "start": "parcel",
   "build": "parcel build"
 }
 ...
}

🚨 Test instructions

I added the integration test to check this behavior

✔️ PR Todo

• Added/updated unit tests for this change
• Filled out test instructions (In case there aren't any unit tests)
• Included links to related issues/PRs

[height]

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

[mischnic]

This would break if you have extensionless entries (which should work fine right now if there's a matching glob in parcelrc).
So maybe something like this instead?

        (await inputFS.stat(entry)).isFile() ? entry : path.join(entry, 'index'),

(and use path.join(entry, 'index') instead of ./. This pattern is used in various places, for example also when determining the projectRoot a few lines further down)

[Shinyaigeek]

Thank you! As you said, it is better to use stat than to judge with extension. I fixed in 4b2d4a6 👍

[mischnic]

See my next comment
Actually, these entries can be entries so we definitely need something like

      entries.map(async entry => {
        // getRootDir treats the input as files, so getRootDir(["/home/user/myproject"]) returns "/home/user".
        // Instead we need to make the entries refer to some file inside the specified folders.
        let isFolder = false;
        try {
          isFolder = (await inputFS.stat(entry)).isDirectory()
        } catch(e) {
          // Ignore error if entry is a glob or doesn't exist, assume it's a file
        }

        return isFolder
          ? path.join(entry, 'index')
          : entry;
      })

And for reference, some cases where something extname-based doesn't work correctly:

path.extname("home/my-project.new/") === '.new'
path.extname("project/*") === ''

[devongovett]

I'm not sure assuming a glob is a file is right either though. If we want to be 100% correct, we'd need to expand the globs first. But that is expensive, and already done in the entry request. This is just duplicating all that again. Ideally we'd re-order this so the root dir gets resolved after entries are resolved. Not sure that's possible though. For finding the project root, a best guess at a starting point to search upward for yarn.lock etc. is probably fine.

[mischnic]

We only need to do one stat call because this problematic behaviour of getRootDir only happens if there is exactly one entrypoint and that entrypoint is the project root folder (and not a glob).

diff --git packages/core/core/src/resolveOptions.js packages/core/core/src/resolveOptions.js
index 42c529792..c26d79b88 100644
--- packages/core/core/src/resolveOptions.js
+++ packages/core/core/src/resolveOptions.js
@@ -14,7 +14,7 @@ import {hashString} from '@parcel/hash';
 import {NodeFS} from '@parcel/fs';
 import {LMDBCache, FSCache} from '@parcel/cache';
 import {NodePackageManager} from '@parcel/package-manager';
-import {getRootDir, relativePath, resolveConfig} from '@parcel/utils';
+import {isGlob, getRootDir, relativePath, resolveConfig} from '@parcel/utils';
 import loadDotEnv from './loadDotEnv';
 import {toProjectPath} from './projectPath';
 import {getResolveFrom} from './requests/ParcelConfigRequest';
@@ -50,6 +50,19 @@ export default async function resolveOptions(
     entries = [path.resolve(inputCwd, initialOptions.entries)];
   }

+  if (entries.length === 1 && !isGlob(entries[0])) {
+    let [entry] = entries;
+    let isDirectory = false;
+    try {
+      isDirectory = (await inputFS.stat(entry)).isDirectory();
+    } catch (e) {
+      // ignore failing stat call
+    }
+    if (isDirectory) {
+      entries[0] = path.join(entry, 'index');
+    }
+  }
+
   let entryRoot = getRootDir(entries);
   let projectRootFile =
     (await resolveConfig(

This prevents unnecessarily many stat calls and works correctly with globs.
Cases for which this approach works correctly:

parcel '/Users/niklas/my-project/'
parcel '/Users/niklas/my-project/*'
parcel '/Users/niklas/my-project/packages'
parcel '/Users/niklas/my-project/package.json'
parcel '/Users/niklas/my-project/packages/*'
parcel '/Users/niklas/my-project/packages/**/*.md'

[Shinyaigeek]

yes, as @mischnic said (and be described in this PR's description), this issue can happen only if the one entry point(and this entry point can not be glob). I think the proposed way is good because this prevents unnecessary stat calls.

[mischnic]

@Shinyaigeek Could you change your implementation to use that approach instead?

[Shinyaigeek]

@mischnic 7c9d5a8 DONE 👍

While I changed the code, I noticed that adding the side-effects to entries will cause a reference error after getRootDir process because arranged entries refer to a non-existing file.

This process is only for getRootDir, so I thought about adding this feature to getRootDir, but this process is needed only when solving entries given by package.json or CLI, so this feature should not be included in getRootDir, parcel’s utility module. so I added a process to change the arguments passed into getRootDir (and not change entries itself).

[mischnic]

Thanks! I forgot to check whether entries was also used in other places 👍