Skip to content

Conversation

@tmoody
Copy link
Member

@tmoody tmoody commented Dec 11, 2025

Issue

  1. @sasjs/cli has some dependencies on @sasjs/core.

When @sasjs/cli is used in pipeline testing of an updated @sasjs/core package it is preferable to use the updated @sasjs/core in place of the cli's node_modules/@sasjs/core.

In a clean environment, with no installed npm packages, an invocation of npx @sasjs/cli with the build argument results in failure as no installation of @sasjs/core is returned by the getNodeModulesPath() function (npx caches the @sasjs/core dependency locally but that temporary node_modules path is not being considered).

  1. The repository is susceptible to npm supply chain attacks (SHA1-HULUD).

Intent

  1. When @sasjs/cli searches for the @sasjs/core folder, first read an environment variable that points at the root directory of a chosen local @sasjs/core package. This permits over-riding the @sasjs/core installed as the cli's node_modules dependency.

If no such environment variable is found, or if it is blank, rely instead on node's require.resolve() routine to determine the required package's node_modules path.

  1. Prevent automatic execution of pre and post npm scripts.

Implementation

  1. src/utils/setConstants.ts has the job of declaring sasjsConstants.macroCorePath as the path to use.
    macroCorePath is now first set from the value of an environment variable (also named macroCorePath). In the even that it is blank a secondary call to getNodeModulePath('@sasjs/core') should return a non-blank value unless the @sasjs/core dependency has accidentally been removed from the package.json.

A blank value for sasjsConstants.macroCorePath is still a possibility and is acceptable for some functionality within @sasjs/cli.
However, for a sasjs command that requests a build, this path is required and processing only continues in build.ts if the value is populated, otherwise an informational error message is thrown and the process halts at an early build stage.

  1. New file .npmrc containing the ignore-scripts=true directive configures npm to skip automatic processing of associated scripts with a pre or post prefix when running a given script name (i.e. given npm install, no script named preinstall or postinstall will also be executed in the same submission)

Checks

  • Code is formatted correctly (npm run lint:fix).
  • Any new functionality has been unit tested.
  • All unit tests are passing (npm test).
  • Unit tests coverage has been increased and a new threshold is set.
  • All CI checks are green.
  • Development comments have been added or updated.
  • Development documentation coverage has been increased and a new threshold is set.
  • Reviewer is assigned.

Reviewer checks

  • Any new code is documented.

@tmoody tmoody self-assigned this Dec 11, 2025
@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Coverage report

Total coverage

Status Category Percentage Covered / Total
🟡 Statements 73.62% 3319/4508
🟡 Branches 60.56% 1319/2178
🟡 Functions 73.74% 674/914
🟢 Lines 82.86% 8459/10209

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

Reduced coverage

Status Filename Statements Branches Functions Lines
🟢 src/utils/utils.ts 73.39% 61.29% 71.74% 81.92%
🟢 src/utils/test.ts 84.66% (-1.14% 🔻) 68.12% 77.14% 92.09%
🟢 src/commands/compile/compileSingleFile.ts 95.83% (-2.08% 🔻) 84% (-4% 🔻) 100% 100%
🟢 src/commands/build/build.ts 87.12% 70.42% 95.45% 89.29% (-1.71% 🔻)
🟢 src/commands/docs/generateDocs.ts 84.51% (-1.41% 🔻) 64.71% (-2.94% 🔻) 100% 90.35%

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from 99bc3ba

mulahasanovic
mulahasanovic previously approved these changes Dec 11, 2025
mulahasanovic
mulahasanovic previously approved these changes Dec 12, 2025
@allanbowe allanbowe merged commit 18c444d into main Dec 16, 2025
1 of 2 checks passed
@allanbowe allanbowe deleted the macroCorePath_envVar branch December 16, 2025 12:24
@github-actions
Copy link

🎉 This PR is included in version 4.12.16 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants