diff --git a/docs/diagrams/sasjs-compile.md b/docs/diagrams/sasjs-compile.md index b62bdac9..db0ce962 100644 --- a/docs/diagrams/sasjs-compile.md +++ b/docs/diagrams/sasjs-compile.md @@ -106,3 +106,11 @@ flowchart TD through `loadDependencies()` → `loadDependenciesFile()` (from `@sasjs/utils`), which is also memoized per-file via the `compileTree` (`{target}_compileTree.json`) to avoid recomputing dependencies across compiles. +- `loadDependencies()` resolves `%macro` calls against `macroFolders`/`programFolders` + (from the target/config) plus `process.sasjsConstants.macroCorePath`, for macros from + `@sasjs/core`. `macroCorePath` is computed once per run in `setConstants()` + (`src/utils/setConstants.ts`), in this order: the `macroCorePath` env var if set, + else `@sasjs/core` resolved starting from `process.projectDir` (the user's own + project - via `getNodeModulePath('@sasjs/core', process.projectDir)`), else + `@sasjs/core` resolved relative to `@sasjs/cli`'s own install location, as a + fallback for projects that haven't installed `@sasjs/core` themselves. diff --git a/src/utils/setConstants.ts b/src/utils/setConstants.ts index cb501ae3..ee5e78e3 100644 --- a/src/utils/setConstants.ts +++ b/src/utils/setConstants.ts @@ -43,17 +43,19 @@ export const setConstants = async ( const buildDestinationJobsFolder = path.join(buildDestinationFolder, 'jobs') const buildDestinationDbFolder = path.join(buildDestinationFolder, 'db') const buildDestinationDocsFolder = path.join(buildDestinationFolder, 'docs') - // Edge case: @sasjs/cli has a dependency on @sasjs/core. - // When @sasjs/cli is used to submit a test of the @sasjs/core - // repo, it is desirable to use that @sasjs/core repo as the dependency rather - // than the older version in @sasjs/cli node_modules. - // To achieve this, set environment variable `macroCorePath` to the root dir - // of the local @sasjs/core package. If found, this takes precedence over - // any node_modules installations of @sasjs/core. + // Resolution order for the @sasjs/core macros used by 'sasjs compile' etc: + // 1. the `macroCorePath` environment variable, if set - an explicit + // override, e.g. for developing against a local @sasjs/core checkout. + // 2. @sasjs/core installed in the user's own project (process.projectDir). + // 3. @sasjs/cli's own @sasjs/core dependency, as a fallback for projects + // that haven't installed @sasjs/core themselves. let macroCorePath = (process.env.macroCorePath as string) ?? '' + + if (macroCorePath === '') { + macroCorePath = await getNodeModulePath('@sasjs/core', process.projectDir) + } + if (macroCorePath === '') { - // If no environment variable is set/populated then check for an installed - // @sasjs/core in locations known to node. macroCorePath = await getNodeModulePath('@sasjs/core') } diff --git a/src/utils/spec/setConstants.spec.ts b/src/utils/spec/setConstants.spec.ts index 542cdf42..15b79056 100644 --- a/src/utils/spec/setConstants.spec.ts +++ b/src/utils/spec/setConstants.spec.ts @@ -80,28 +80,40 @@ describe('setConstants', () => { verifySasjsConstants(undefined, false, false) }) - test('should call getNodeModulePath once when environment variable macroCorePath is undefined', async () => { + test('should look up @sasjs/core scoped to process.projectDir, then fall back unscoped, when environment variable macroCorePath is undefined', async () => { process.env.macroCorePath = undefined const getNodeModulePathSpy = jest .spyOn(utils, 'getNodeModulePath') - .mockImplementation(async (packageName: string) => Promise.resolve('')) + .mockImplementation(async () => Promise.resolve('')) await setConstants() - expect(getNodeModulePathSpy).toHaveBeenCalledOnceWith('@sasjs/core') + expect(getNodeModulePathSpy).toHaveBeenNthCalledWith( + 1, + '@sasjs/core', + process.projectDir + ) + expect(getNodeModulePathSpy).toHaveBeenNthCalledWith(2, '@sasjs/core') + expect(getNodeModulePathSpy).toHaveBeenCalledTimes(2) }) - test('should call getNodeModulePath once when environment variable macroCorePath is blank', async () => { + test('should look up @sasjs/core scoped to process.projectDir, then fall back unscoped, when environment variable macroCorePath is blank', async () => { process.env.macroCorePath = '' const getNodeModulePathSpy = jest .spyOn(utils, 'getNodeModulePath') - .mockImplementation(async (packageName: string) => Promise.resolve('')) + .mockImplementation(async () => Promise.resolve('')) await setConstants() - expect(getNodeModulePathSpy).toHaveBeenCalledOnceWith('@sasjs/core') + expect(getNodeModulePathSpy).toHaveBeenNthCalledWith( + 1, + '@sasjs/core', + process.projectDir + ) + expect(getNodeModulePathSpy).toHaveBeenNthCalledWith(2, '@sasjs/core') + expect(getNodeModulePathSpy).toHaveBeenCalledTimes(2) }) test('should not call getNodeModulePath when environment variable macroCorePath is populated', async () => { @@ -115,6 +127,51 @@ describe('setConstants', () => { expect(getNodeModulePathSpy).toBeCalledTimes(0) }) + + test('should prefer @sasjs/core resolved relative to process.projectDir over the CLI-relative fallback', async () => { + process.env.macroCorePath = undefined + const projectCorePath = path.join( + 'some', + 'project', + 'node_modules', + '@sasjs', + 'core' + ) + + const getNodeModulePathSpy = jest + .spyOn(utils, 'getNodeModulePath') + .mockImplementation(async (_packageName: string, fromDir?: string) => + Promise.resolve(fromDir ? projectCorePath : 'cli-relative-core-path') + ) + + await setConstants() + + // found on the first, project-scoped lookup - the unscoped fallback + // should never be reached + expect(getNodeModulePathSpy).toHaveBeenCalledTimes(1) + expect(process.sasjsConstants.macroCorePath).toEqual(projectCorePath) + }) + + test('should fall back to the CLI-relative @sasjs/core when the project has none installed', async () => { + process.env.macroCorePath = undefined + const fallbackCorePath = path.join('cli', 'node_modules', '@sasjs', 'core') + + const getNodeModulePathSpy = jest + .spyOn(utils, 'getNodeModulePath') + .mockImplementation(async (_packageName: string, fromDir?: string) => + Promise.resolve(fromDir ? '' : fallbackCorePath) + ) + + await setConstants() + + expect(getNodeModulePathSpy).toHaveBeenNthCalledWith( + 1, + '@sasjs/core', + process.projectDir + ) + expect(getNodeModulePathSpy).toHaveBeenNthCalledWith(2, '@sasjs/core') + expect(process.sasjsConstants.macroCorePath).toEqual(fallbackCorePath) + }) }) const verifySasjsConstants = ( diff --git a/src/utils/spec/utils.spec.ts b/src/utils/spec/utils.spec.ts index 66086031..4670cf4b 100644 --- a/src/utils/spec/utils.spec.ts +++ b/src/utils/spec/utils.spec.ts @@ -444,6 +444,36 @@ describe('utils', () => { getNodeModulePath('sasjs-nonexistent-module-xyz') ).resolves.toEqual('') }) + + it('should prefer a module installed under the given fromDir over default resolution', async () => { + const fakeModuleName = `sasjs-test-fake-module-${generateTimestamp()}` + const fromDir = path.join( + require('os').tmpdir(), + `getNodeModulePath-${generateTimestamp()}` + ) + const fakeModuleDir = path.join(fromDir, 'node_modules', fakeModuleName) + + await createFolder(fakeModuleDir) + await createFile( + path.join(fakeModuleDir, 'package.json'), + JSON.stringify({ name: fakeModuleName, version: '1.0.0' }) + ) + + // not resolvable at all without fromDir - it only exists in fromDir's + // own node_modules, nowhere on the default resolution path + await expect(getNodeModulePath(fakeModuleName)).resolves.toEqual('') + + const resolvedPath = await getNodeModulePath(fakeModuleName, fromDir) + + // compare tails rather than exact paths: require.resolve returns the + // real (symlink-resolved) path, which can differ from os.tmpdir()'s + // raw value (e.g. /var vs /private/var on macOS) + expect( + resolvedPath.endsWith(path.join('node_modules', fakeModuleName)) + ).toEqual(true) + + await deleteFolder(fromDir) + }) }) describe('getUniqServicesObj', () => { diff --git a/src/utils/utils.ts b/src/utils/utils.ts index ac8f7e37..9c7bcb81 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -573,12 +573,37 @@ export const isSASjsProject = async () => { return false } -export const getNodeModulePath = async (module: string): Promise => { +/** + * Locates an installed npm module and returns its root folder. + * + * Without `fromDir`, `require.resolve` searches relative to *this file's own* + * location on disk - i.e. wherever @sasjs/cli itself is installed - not the + * user's project. That's fine for CLI-internal dependencies, but wrong for + * anything the user is expected to manage in their own project's + * node_modules (e.g. @sasjs/core): since @sasjs/cli also depends on that + * same package, default resolution finds the CLI's own bundled copy first + * and never reaches the project's. Passing `fromDir` anchors the search at + * that directory instead (Node's `paths` option replaces, rather than + * extends, the default resolution paths), so callers that need + * project-first resolution should pass `process.projectDir` and fall back to + * an unscoped call if that returns nothing. + * @param {string} module - the name of the npm module to locate. + * @param {string} fromDir - optional directory to resolve `module` from, + * instead of this file's own location. + */ +export const getNodeModulePath = async ( + module: string, + fromDir?: string +): Promise => { // Look for ${module}/package.json, then return only the path try { const nodePackagePath = path.dirname( - require.resolve(path.join(module, 'package.json')) + require.resolve( + path.join(module, 'package.json'), + fromDir ? { paths: [fromDir] } : undefined + ) ) + if (nodePackagePath) return nodePackagePath } catch (e: any) { if (e.code !== 'MODULE_NOT_FOUND') throw e