openzeppelin-contracts icon indicating copy to clipboard operation
openzeppelin-contracts copied to clipboard

Bug: scripts/generate/run.js fails if target directories do not exist

Open EricForgy opened this issue 9 months ago • 1 comments

Environment

  • OpenZeppelin Contracts Upgradeable v5.3.0
  • Node.js (Edit: was on v20+, but upgraded to LTS v22.15.1 with same issue)
  • Using npm and Hardhat on macOS
  • Fresh clone of openzeppelin-contracts-upgradeable

Details

When running npm run generate on a fresh clone of the repo, the script fails with an ENOENT error if the target directory (e.g., contracts/utils/math/) doesn’t already exist.

This happens because fs.writeFileSync() is called without ensuring the output directory is present.


Code to reproduce bug

  1. Clone the repo:

    git clone https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable.git
    cd openzeppelin-contracts-upgradeable
    
  2. Install dependencies:

    npm install
    
  3. Run the generator:

    npm run generate
    
  4. You’ll see:

    Error: ENOENT: no such file or directory, open 'contracts/utils/math/SafeCast.sol'
    

Suggested Fix

Add this line to scripts/generate/run.js before writing output:

fs.mkdirSync(path.dirname(output), { recursive: true });

This ensures the parent directories exist before attempting to write files.

Happy to open a PR if this direction makes sense!

EricForgy avatar May 12 '25 01:05 EricForgy

Quick follow-up: after fixing the directory creation issue in run.js, I ran into a second problem when trying to commit the fix — the ESLint pre-commit hook fails with:

TypeError [ERR_INVALID_ARG_TYPE]: The "paths[0]" argument must be of type string. Received undefined

This happens because the ESLint config uses import.meta.dirname, which is only available in Node.js v20.11.0+ (I'm on LTS v22.15.0) and can still be undefined depending on how the config is loaded.

Suggested fix: use the portable fallback:

import { fileURLToPath } from 'url';
const __dirname = path.dirname(fileURLToPath(import.meta.url));

This ensures the config works reliably across environments and avoids crashing the pre-commit hook.

Happy to include this fix in the same PR if helpful. Let me know!

PS: Here is a draft ready to submit: https://github.com/EricForgy/openzeppelin-contracts-upgradeable/commit/38ed3818d610e8f5b342073da6b1e03e4375b9aa

EricForgy avatar May 12 '25 02:05 EricForgy