From 9545ff6147cffba65cabf254368c43b992bc903f Mon Sep 17 00:00:00 2001 From: Kristaps Fabians Geikins Date: Tue, 5 Aug 2025 12:54:53 +0300 Subject: [PATCH 1/2] fix(shared): parseFeatureFlags should ignore empty string values (#5179) * fix(shared): parseFeatureFlags should ignore empty string values * updated values.yaml * updated values.yaml * tests fix --- .../unit/services/viewerResources.spec.ts | 0 packages/shared/src/environment/index.spec.ts | 125 ++++++++++++++++++ packages/shared/src/environment/index.ts | 10 ++ utils/helm/speckle-server/values.schema.json | 5 + utils/helm/speckle-server/values.yaml | 2 + 5 files changed, 142 insertions(+) create mode 100644 packages/server/modules/viewer/tests/unit/services/viewerResources.spec.ts create mode 100644 packages/shared/src/environment/index.spec.ts diff --git a/packages/server/modules/viewer/tests/unit/services/viewerResources.spec.ts b/packages/server/modules/viewer/tests/unit/services/viewerResources.spec.ts new file mode 100644 index 000000000..e69de29bb diff --git a/packages/shared/src/environment/index.spec.ts b/packages/shared/src/environment/index.spec.ts new file mode 100644 index 000000000..4d8184e07 --- /dev/null +++ b/packages/shared/src/environment/index.spec.ts @@ -0,0 +1,125 @@ +import { describe, it, expect, afterEach, beforeEach } from 'vitest' +import { type FeatureFlags, parseFeatureFlags } from './index.js' + +const originalDisableAllFfs = process.env.DISABLE_ALL_FFS || '' +const originalEnableAllFfs = process.env.ENABLE_ALL_FFS || '' + +const setDisableAllFfs = (value: boolean) => { + process.env.DISABLE_ALL_FFS = value.toString() +} + +const setEnableAllFfs = (value: boolean) => { + process.env.ENABLE_ALL_FFS = value.toString() +} + +const resetEnv = () => { + process.env.DISABLE_ALL_FFS = originalDisableAllFfs + process.env.ENABLE_ALL_FFS = originalEnableAllFfs +} + +describe('parseFeatureFlags', () => { + beforeEach(() => { + // Disable global "ALL FFs"/"NO FFs" modes for these tests, cause they break em + setDisableAllFfs(false) + setEnableAllFfs(false) + }) + + afterEach(() => { + resetEnv() + }) + + it('returns all defaults as false', () => { + const flags = parseFeatureFlags({}) + for (const key of Object.keys(flags)) { + expect(flags[key as keyof FeatureFlags]).toBe(false) + } + }) + + it('parses explicit true/false values', () => { + const flags = parseFeatureFlags({ + FF_AUTOMATE_MODULE_ENABLED: 'true', + FF_GENDOAI_MODULE_ENABLED: 'false', + FF_SAVED_VIEWS_ENABLED: 'true' + }) + expect(flags.FF_AUTOMATE_MODULE_ENABLED).toBe(true) + expect(flags.FF_GENDOAI_MODULE_ENABLED).toBe(false) + expect(flags.FF_SAVED_VIEWS_ENABLED).toBe(true) + }) + + it('DISABLE_ALL_FFS disables all flags unless forceInputs is true', () => { + setDisableAllFfs(true) + const flags = parseFeatureFlags( + { + FF_AUTOMATE_MODULE_ENABLED: 'true', + FF_SAVED_VIEWS_ENABLED: 'true' + }, + { forceInputs: false } + ) + for (const key of Object.keys(flags)) { + expect(flags[key as keyof FeatureFlags]).toBe(false) + } + }) + + it('ENABLE_ALL_FFS enables all flags unless forceInputs is true', () => { + setEnableAllFfs(true) + const flags = parseFeatureFlags( + { + FF_AUTOMATE_MODULE_ENABLED: 'false', + FF_SAVED_VIEWS_ENABLED: 'false' + }, + { forceInputs: false } + ) + for (const key of Object.keys(flags)) { + expect(flags[key as keyof FeatureFlags]).toBe(true) + } + }) + + it('forceInputs=true preserves explicit input values even with DISABLE_ALL_FFS', () => { + setDisableAllFfs(true) + const flags = parseFeatureFlags( + { + FF_AUTOMATE_MODULE_ENABLED: 'true', + FF_SAVED_VIEWS_ENABLED: 'false' + }, + { forceInputs: true } + ) + + expect(flags.FF_AUTOMATE_MODULE_ENABLED).toBe(true) + expect(flags.FF_SAVED_VIEWS_ENABLED).toBe(false) + // All others should be false + for (const key of Object.keys(flags)) { + if (key !== 'FF_AUTOMATE_MODULE_ENABLED' && key !== 'FF_SAVED_VIEWS_ENABLED') { + expect(flags[key as keyof FeatureFlags]).toBe(false) + } + } + }) + + it('forceInputs=true preserves explicit input values even with ENABLE_ALL_FFS', () => { + setEnableAllFfs(true) + const flags = parseFeatureFlags( + { + FF_AUTOMATE_MODULE_ENABLED: 'false', + FF_SAVED_VIEWS_ENABLED: 'true' + }, + { forceInputs: true } + ) + + expect(flags.FF_AUTOMATE_MODULE_ENABLED).toBe(false) + expect(flags.FF_SAVED_VIEWS_ENABLED).toBe(true) + // All others should be true + for (const key of Object.keys(flags)) { + if (key !== 'FF_AUTOMATE_MODULE_ENABLED' && key !== 'FF_SAVED_VIEWS_ENABLED') { + expect(flags[key as keyof FeatureFlags]).toBe(true) + } + } + }) + + it('it can handle empty string env vars', () => { + const flags = parseFeatureFlags({ + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error + FF_AUTOMATE_MODULE_ENABLED: '' + }) + expect(flags.FF_AUTOMATE_MODULE_ENABLED).toBe(false) + }) +}) diff --git a/packages/shared/src/environment/index.ts b/packages/shared/src/environment/index.ts index 0ec020756..cbdb8573c 100644 --- a/packages/shared/src/environment/index.ts +++ b/packages/shared/src/environment/index.ts @@ -25,6 +25,16 @@ export const parseFeatureFlags = ( ): FeatureFlags => { const { forceInputs = true } = options || {} + // Clean up input: unset empty values + for (const key of Object.keys(input)) { + const typedKey = key as keyof FeatureFlags + const typedVal = input[typedKey] as unknown + + if (typedVal === undefined || typedVal === '') { + delete input[typedKey] + } + } + //INFO // As a convention all feature flags should be prefixed with a FF_ const res = parseEnv(input, { diff --git a/utils/helm/speckle-server/values.schema.json b/utils/helm/speckle-server/values.schema.json index 1440ebe71..ba95608f5 100644 --- a/utils/helm/speckle-server/values.schema.json +++ b/utils/helm/speckle-server/values.schema.json @@ -134,6 +134,11 @@ "type": "boolean", "description": "Enables the dedicated Rhino based file importer. This is not part of the deployment.", "default": false + }, + "savedViewsEnabled": { + "type": "boolean", + "description": "Enables the ability to create and manage saved views", + "default": false } } }, diff --git a/utils/helm/speckle-server/values.yaml b/utils/helm/speckle-server/values.yaml index 2e4ec73e3..ef6c8b43d 100644 --- a/utils/helm/speckle-server/values.yaml +++ b/utils/helm/speckle-server/values.yaml @@ -75,6 +75,8 @@ featureFlags: backgroundJobsEnabled: false ## @param featureFlags.rhinoFileImporterEnabled Enables the dedicated Rhino based file importer. This is not part of the deployment. rhinoFileImporterEnabled: false + ## @param featureFlags.savedViewsEnabled Enables the ability to create and manage saved views + savedViewsEnabled: false analytics: ## @param analytics.enabled Enable or disable analytics From 902c290a63f22f575fc344ecef806a3603b987a8 Mon Sep 17 00:00:00 2001 From: Kristaps Fabians Geikins Date: Tue, 5 Aug 2025 13:20:32 +0300 Subject: [PATCH 2/2] fix(server): attempting to import .d.ts files (#5180) --- packages/server/.c8rc.json | 3 ++- packages/server/esmLoader.js | 2 +- .../viewer/domain/operations/{resources.d.ts => resources.ts} | 0 .../domain/operations/{savedViews.d.ts => savedViews.ts} | 0 .../viewer/domain/types/{resources.d.ts => resources.ts} | 0 .../viewer/domain/types/{savedViews.d.ts => savedViews.ts} | 0 6 files changed, 3 insertions(+), 2 deletions(-) rename packages/server/modules/viewer/domain/operations/{resources.d.ts => resources.ts} (100%) rename packages/server/modules/viewer/domain/operations/{savedViews.d.ts => savedViews.ts} (100%) rename packages/server/modules/viewer/domain/types/{resources.d.ts => resources.ts} (100%) rename packages/server/modules/viewer/domain/types/{savedViews.d.ts => savedViews.ts} (100%) diff --git a/packages/server/.c8rc.json b/packages/server/.c8rc.json index 4e4115670..5ef7f50b4 100644 --- a/packages/server/.c8rc.json +++ b/packages/server/.c8rc.json @@ -16,6 +16,7 @@ "**/{ava,babel,nyc}.config.{js,cjs,mjs}", "**/jest.config.{js,cjs,mjs,ts}", "**/{karma,rollup,webpack}.config.js", - "**/.{eslint,mocha}rc.{js,cjs}" + "**/.{eslint,mocha}rc.{js,cjs}", + "**/domain/**/*.{ts,js}" ] } diff --git a/packages/server/esmLoader.js b/packages/server/esmLoader.js index 49c39c568..652f09780 100644 --- a/packages/server/esmLoader.js +++ b/packages/server/esmLoader.js @@ -21,7 +21,7 @@ const aliases = { /** * EXTENSIONS TO EVALUATE FOR EXTENSIONLESS IMPORTS */ -const extensions = ['.ts', '.js', '.mjs', '.cjs', '.json', '.d.ts'] +const extensions = ['.ts', '.js', '.mjs', '.cjs', '.json'] // Register the module hooks register('./esmLoader.js', { diff --git a/packages/server/modules/viewer/domain/operations/resources.d.ts b/packages/server/modules/viewer/domain/operations/resources.ts similarity index 100% rename from packages/server/modules/viewer/domain/operations/resources.d.ts rename to packages/server/modules/viewer/domain/operations/resources.ts diff --git a/packages/server/modules/viewer/domain/operations/savedViews.d.ts b/packages/server/modules/viewer/domain/operations/savedViews.ts similarity index 100% rename from packages/server/modules/viewer/domain/operations/savedViews.d.ts rename to packages/server/modules/viewer/domain/operations/savedViews.ts diff --git a/packages/server/modules/viewer/domain/types/resources.d.ts b/packages/server/modules/viewer/domain/types/resources.ts similarity index 100% rename from packages/server/modules/viewer/domain/types/resources.d.ts rename to packages/server/modules/viewer/domain/types/resources.ts diff --git a/packages/server/modules/viewer/domain/types/savedViews.d.ts b/packages/server/modules/viewer/domain/types/savedViews.ts similarity index 100% rename from packages/server/modules/viewer/domain/types/savedViews.d.ts rename to packages/server/modules/viewer/domain/types/savedViews.ts