Skip to content

Commit

Permalink
Fix require load of CJS config from ESM module (#6458)
Browse files Browse the repository at this point in the history
  • Loading branch information
sneridagh authored Nov 2, 2024
1 parent c6a522d commit c9e6264
Show file tree
Hide file tree
Showing 16 changed files with 184 additions and 153 deletions.
4 changes: 2 additions & 2 deletions packages/registry/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ docs-rtd-registry: ## Build Plone Registry docs on RTD

## Build

.PHONY: rename-to-cjs
rename-to-cjs: ## Rename the built files js -> cjs
.PHONY: fix-build
fix-build: ## Rename the built files js -> cjs and fix import.meta.url appearences in cjs
mv dist/cjs/addon-registry.js dist/cjs/addon-registry.cjs
mv dist/cjs/create-addons-loader.js dist/cjs/create-addons-loader.cjs
mv dist/cjs/create-theme-loader.js dist/cjs/create-theme-loader.cjs
3 changes: 2 additions & 1 deletion packages/registry/__tests__/fixtures/test-app/package.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"name": "test-app"
"name": "test-app",
"type": "module"
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module.exports = {
addons: ['@plone/slots', 'my-addon'],
};
const addons = ['@plone/slots', 'my-addon'];

export { addons };

This file was deleted.

This file was deleted.

1 change: 1 addition & 0 deletions packages/registry/news/6458.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ERR_REQUIRE from ESM module requiring CJS module. @sneridagh
1 change: 1 addition & 0 deletions packages/registry/news/6458.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow any type `js`, `cjs`, `mjs`, `ts` as configuration for the add-on registry. @sneridagh
3 changes: 2 additions & 1 deletion packages/registry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"build": "parcel build && pnpm build:node:esm && pnpm build:node:cjs",
"build:force": "rm -rf dist && parcel build --no-cache && pnpm build:node:esm && pnpm build:node:cjs",
"build:node:esm": "tsc --project tsconfig.node.json || true",
"build:node:cjs": "tsc --project tsconfig.node.json --module commonjs --moduleResolution Node --outDir dist/cjs || true && make rename-to-cjs",
"build:node:cjs": "tsc --project tsconfig.node.json --module commonjs --moduleResolution Node --outDir dist/cjs || true && make fix-build",
"test": "vitest",
"test:debug": "vitest --inspect-brk --no-file-parallelism registry",
"dry-release": "release-it --dry-run",
Expand All @@ -91,6 +91,7 @@
}
},
"dependencies": {
"auto-config-loader": "^1.7.7",
"crypto-random-string": "3.2.0",
"debug": "4.3.2",
"dependency-graph": "0.10.0",
Expand Down
78 changes: 54 additions & 24 deletions packages/registry/src/addon-registry/addon-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import path from 'path';
import fs from 'fs';
import _debug from 'debug';
import { DepGraph } from 'dependency-graph';
import { createRequire } from 'node:module';
import { autoConf, jsLoader, getConfigPath } from 'auto-config-loader';

const debug = _debug('shadowing');
const debugShadowing = _debug('shadowing');
const debugConfig = _debug('config');

export type Package = {
name: string;
Expand Down Expand Up @@ -247,34 +250,55 @@ class AddonRegistry {

isESM = () => this.packageJson.type === 'module';

/**
* Gets the registry configuration from the project's config file (.js, .cjs, .ts, .mts).
* It tries first if there's an environment variable pointing to the config file.
* If it doesn't exist, it tries to load one from the local project.
* If it doesn't exist, it returns an empty config object.
*/
getRegistryConfig(projectRootPath: string) {
let config: VoltoConfigJS = {
const config: VoltoConfigJS = {
addons: [],
theme: '',
};
const CONFIGMAP = {
REGISTRYCONFIG: this.isESM()
? 'registry.config.cjs'
: 'registry.config.js',
VOLTOCONFIG: this.isESM() ? 'volto.config.cjs' : 'volto.config.js',
};

for (const key in CONFIGMAP) {
if (process.env[key]) {
const resolvedPath = path.resolve(process.env[key]);
if (fs.existsSync(resolvedPath)) {
const voltoConfigPath = resolvedPath;
console.log(`Using configuration file in: ${voltoConfigPath}`);
config = require(voltoConfigPath);
break;
function loadConfigFromEnvVar() {
let config: VoltoConfigJS | null = null;
const ENVVARCONFIG = ['REGISTRYCONFIG', 'VOLTOCONFIG'];
ENVVARCONFIG.forEach((envVar) => {
if (process.env[envVar]) {
const resolvedPath = path.resolve(process.env[envVar]);
if (fs.existsSync(resolvedPath)) {
// @ts-expect-error It seems that the types are not correct in the auto-config-loader
config = jsLoader(resolvedPath);
debugConfig(
`[@plone/registry] Using configuration file in: ${resolvedPath}`,
);
}
}
} else if (fs.existsSync(path.join(projectRootPath, CONFIGMAP[key]))) {
config = require(path.join(projectRootPath, CONFIGMAP[key]));
break;
}
});

return config;
}

return config;
function loadConfigFromNamespace(namespace: string) {
let config: VoltoConfigJS | null = null;
config = autoConf(namespace, {
cwd: projectRootPath,
mustExist: true, // It seems that the bool is inverted
});
debugConfig(
`[@plone/registry] Using configuration file in: ${getConfigPath()}`,
);
return config;
}

return (
loadConfigFromEnvVar() ||
loadConfigFromNamespace('registry') ||
loadConfigFromNamespace('volto') ||
config
);
}

/**
Expand All @@ -297,8 +321,13 @@ class AddonRegistry {
const jsConfig = JSON.parse(
fs.readFileSync(configFile, 'utf-8'),
).compilerOptions;
pathsConfig = jsConfig.paths;
baseUrl = jsConfig.baseUrl;
if (jsConfig) {
pathsConfig = jsConfig.paths;
baseUrl = jsConfig.baseUrl;
} else {
pathsConfig = {};
baseUrl = '';
}
}

return [baseUrl, pathsConfig];
Expand Down Expand Up @@ -375,6 +404,7 @@ class AddonRegistry {
}

if (!Object.keys(this.packages).includes(name)) {
const require = createRequire(this.projectRootPath);
const resolved = require.resolve(name, { paths: [this.projectRootPath] });
const basePath = getPackageBasePath(resolved);
const packageJson = path.join(basePath, 'package.json');
Expand Down Expand Up @@ -674,7 +704,7 @@ class AddonRegistry {
.replace(/\.(js|jsx|ts|tsx)$/, '')
] = path.resolve(filename);
} else {
debug(
debugShadowing(
`The file ${filename} doesn't exist in the ${name} (${targetPath}), unable to customize.`,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export function createAddonsLoader(
// the `tempInProject` allows to place it inside
let addonsLoaderPath: string;
if (tempInProject) {
addonsLoaderPath = path.join(process.cwd(), 'src', '.addons-loader.js');
addonsLoaderPath = path.join(process.cwd(), '.registry.loader.js');
} else {
addonsLoaderPath = tmp.tmpNameSync({ postfix: '.js' });
}
Expand Down
1 change: 1 addition & 0 deletions packages/registry/vite-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const PloneRegistryVitePlugin = () => {
const addonsLoaderPath = createAddonsLoader(
registry.getAddonDependencies(),
registry.getAddons(),
{ tempInProject: true },
);

const [addonsThemeLoaderVariablesPath, addonsThemeLoaderMainPath] =
Expand Down
26 changes: 0 additions & 26 deletions packages/volto/__tests__/addon-registry-project.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,29 +241,3 @@ describe('Addon via env var - Released addon', () => {
).toBe(true);
});
});

describe('Addon via env var - local packages folder addon', () => {
const originalEnv = process.env;

beforeEach(() => {
jest.resetModules();
process.env = {
...originalEnv,
ADDONS: 'test-local-packages-via-addons-env-var',
};
});

afterEach(() => {
process.env = originalEnv;
});

it('addons can be specified on the fly using ADDONS env var - local packages folder addon', () => {
const base = path.join(__dirname, 'fixtures', 'test-volto-project');
const { registry } = AddonRegistry.init(base);
expect(
Object.keys(registry.packages).includes(
'test-local-packages-via-addons-env-var',
),
).toBe(true);
});
});

This file was deleted.

This file was deleted.

1 change: 1 addition & 0 deletions packages/volto/news/6458.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ERR_REQUIRE from ESM module requiring CJS module in `@plone/registry` fix tests. @sneridagh
Loading

0 comments on commit c9e6264

Please sign in to comment.