[Bug?]: Non .ts files in scripts/ with same name as script break exec

[Benjamin-Lee]

What's not working?

Lets I have a script called foo in scripts/. foo.ts requires a data file foo.json, which it imports. yarn rw exec foo now breaks. The error reference is 04eeca2b-0df4-4e56-8b67-3c9d7523802b.

As a fix, one can easily rename the data file.

How do we reproduce the bug?

No response

What's your environment? (If it applies)

System:
    OS: macOS 13.3
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 20.5.0 - /private/var/folders/gw/sk03t1cn3wlgclyc55yst44w0000gn/T/xfs-b539a9b4/node
    Yarn: 3.6.1 - /private/var/folders/gw/sk03t1cn3wlgclyc55yst44w0000gn/T/xfs-b539a9b4/yarn
  Databases:
    SQLite: 3.39.5 - /usr/bin/sqlite3
  Browsers:
    Chrome: 117.0.5938.92
    Safari: 16.4
  npmPackages:
    @redwoodjs/auth-dbauth-setup: 6.2.1 => 6.2.1 
    @redwoodjs/cli-storybook: 6.2.1 => 6.2.1 
    @redwoodjs/core: 6.2.1 => 6.2.1

Are you interested in working on this?

• I'm interested in working on this

[dac09]

Hi @Benjamin-Lee, thanks for reporting this! I haven't reproduced this, but can totally see why it happens.

The issue is because we're using require.resolve in this line here: https://github.com/redwoodjs/redwood/blob/main/packages/cli/src/commands/execHandler.js#L103 - which would pick up the json file ofcourse. Something similar probably happens when trying to actually execute: https://github.com/redwoodjs/redwood/blob/main/packages/cli/src/lib/exec.js#L14

Off the top of my head, I can't think of a clean way to make it ignore the other files, but happy to hear suggestions before you start!

Things to note:

• we need to support .js, .jsx, .tsand.tsx` I think
• you can totally avoid using require.resolve

[Tobbe]

Hey @Benjamin-Lee have you had a chance to look at a fix for this yet? Are you still interested? No pressure, just wanted an update 🙂 Have a great day!

[codersmith]

Hi @Tobbe and @Benjamin-Lee, I'd like to pick this up if it's alright with you? This would be my first time contributing so I may need some guidance.

It seems like there are two problems/requirements:

1. The handler needs to ensure there aren't ambiguous scripts defined such as [foo.js, foo.ts]
2. If someone wants to include other files like foo.json or foo.whatever, it should have no impact on their script named foo.{js,jsx,ts,tsx}.

For the first, the execHandler is currently using require.resolve to check that the script actually exists before trying to run it. We can use the file system to do this instead. Something like this:

  // - execHandler.js
  const tgtScriptName = path.parse(scriptPath)?.name
  const tgtMatch = findScripts()
    .map((p) => path.parse(p)?.name)
    .filter((n) => n == tgtScriptName)

With this, we'd expect tgtMatch to contain exactly one matching script with the extension *.{js,jsx,ts,tsx}. If it has zero, or more than 1, then it's a problem.

For the second requirement, we can use a similar approach to first resolve the full script path including file extension, and pass this in to the call to require. That will prevent require from accidentally loading foo.json when it should have loaded foo.js. Something along these lines:

// - files.ts
export const resolveJSPath = (pathNoExt: string) => {
  const files = fg.sync(`${pathNoExt}.{js,jsx,ts,tsx}`, {
    absolute: true,
    ignore: ['node_modules'],
  })
  if (files.length == 1) {
    return files[0]
  }
  return false
}

// - exec.js
const script = require(resolveJSPath(scriptPath))

Questions:

1. Does this approach seem directionally on track?
2. Any gotchas or special cases I should be on the lookout for?