Skip to content

Commit cc266fe

Browse files
committed
update validation
1 parent a12858d commit cc266fe

File tree

5 files changed

+204
-30
lines changed

5 files changed

+204
-30
lines changed

docs/PLATFORM_ISOLATION.md

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ export function getWindowSize() {
3838

3939
Valid platform identifiers: `'browser'`, `'node'`, `'react_native'`, `'__universal__'`
4040

41+
**Important**: Only files that explicitly include `'__universal__'` in their `__platforms` array are considered universal. Files that list all concrete platforms (e.g., `['browser', 'node', 'react_native']`) are treated as multi-platform files, NOT universal files. They must still ensure imports support all their declared platforms.
42+
4143
### File Naming Convention (Optional)
4244

4345
While not enforced, you may optionally use file name suffixes for clarity:
@@ -63,14 +65,29 @@ A file is compatible if:
6365

6466
### Compatibility Examples
6567

68+
**Core Principle**: When file A imports file B, file B must support ALL platforms that file A runs on.
69+
70+
**Universal File (`__platforms = ['__universal__']`)**
71+
- ✅ Can import from: universal files (with `__universal__`)
72+
- ❌ Cannot import from: any platform-specific files, even `['browser', 'node', 'react_native']`
73+
- **Why**: Universal files run everywhere, so all imports must explicitly be universal
74+
- **Note**: Listing all platforms like `['browser', 'node', 'react_native']` is NOT considered universal
75+
6676
**Single Platform File (`__platforms = ['browser']`)**
67-
- ✅ Can import from: universal files, files with `['browser']` or `['browser', 'react_native']`
68-
- ❌ Cannot import from: files with `['node']` or `['react_native']` only
77+
- ✅ Can import from: universal files, files with `['browser']`, multi-platform files that include browser like `['browser', 'react_native']`
78+
- ❌ Cannot import from: files without browser support like `['node']` or `['react_native']` only
79+
- **Why**: The import must support the browser platform
6980

7081
**Multi-Platform File (`__platforms = ['browser', 'react_native']`)**
71-
- ✅ Can import from: universal files, files with exactly `['browser', 'react_native']`
72-
- ❌ Cannot import from: `.browser.ts` (browser only), `.react_native.ts` (react_native only), `.node.ts`
73-
- **Why?** A file supporting both platforms needs imports that work in BOTH environments
82+
- ✅ Can import from: universal files, files with `['browser', 'react_native']`, supersets like `['browser', 'node', 'react_native']`
83+
- ❌ Cannot import from: files missing any platform like `['browser']` only or `['node']`
84+
- **Why**: The import must support BOTH browser AND react_native
85+
86+
**All-Platforms File (`__platforms = ['browser', 'node', 'react_native']`)**
87+
- ✅ Can import from: universal files, files with exactly `['browser', 'node', 'react_native']`
88+
- ❌ Cannot import from: any subset like `['browser']`, `['browser', 'react_native']`, etc.
89+
- **Why**: This is NOT considered universal - imports must support all three platforms
90+
- **Note**: If your code truly works everywhere, use `['__universal__']` instead
7491

7592
### Examples
7693

@@ -120,11 +137,18 @@ import { NodeRequestHandler } from './utils/http_request_handler/request_handler
120137
import { getWindowSize } from './utils/web-features'; // ❌ Not compatible with Node
121138
```
122139

140+
```typescript
141+
// In lib/shared_types.ts (Universal file)
142+
// export const __platforms = ['__universal__'];
143+
144+
import { helper } from './helper.browser'; // ❌ Browser-only, universal file needs imports that work everywhere
145+
```
146+
123147
```typescript
124148
// In lib/utils/web-api.ts
125149
// export const __platforms = ['browser', 'react_native'];
126150

127-
// If helper.browser.ts is browser-only (no __platforms export)
151+
// If helper.browser.ts is browser-only
128152
import { helper } from './helper.browser'; // ❌ Browser-only, doesn't support react_native
129153

130154
// This file needs imports that work in BOTH browser AND react_native
@@ -150,9 +174,10 @@ The validation script (`scripts/validate-platform-isolation-ts.js`):
150174

151175
1. Scans all source files in the `lib/` directory (excluding tests)
152176
2. **Verifies every file has a `__platforms` export** - fails immediately if any file is missing this
153-
3. Parses import statements using TypeScript AST (ES6 imports, require, dynamic imports)
154-
4. Checks that each import follows the platform isolation rules based on declared platforms
155-
5. Fails the build if violations are found or if any file lacks `__platforms` export
177+
3. **Validates all platform values** - ensures values in `__platforms` arrays are valid (read from Platform type)
178+
4. Parses import statements using TypeScript AST (ES6 imports, require, dynamic imports)
179+
5. **Checks import compatibility**: For each import, verifies that the imported file supports ALL platforms that the importing file runs on
180+
6. Fails the build if violations are found or if any file lacks `__platforms` export
156181

157182
**ESLint Integration:** The `require-platform-declaration` ESLint rule also enforces the `__platforms` export requirement during development.
158183

lib/core/custom_attribute_condition_evaluator/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
* See the License for the specific language governing permissions and *
1414
* limitations under the License. *
1515
***************************************************************************/
16+
export const __platforms = ['__universal__'];
17+
1618
import { Condition, OptimizelyUserContext } from '../../shared_types';
1719

1820
import fns from '../../utils/fns';

lib/utils/event_tag_utils/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
export const __platforms = ['__universal__'];
17+
1618
import {
1719
FAILED_TO_PARSE_REVENUE,
1820
FAILED_TO_PARSE_VALUE,

scripts/README.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,14 @@ npm run build
2424
The script:
2525
1. Scans all TypeScript/JavaScript files in the `lib/` directory
2626
2. **Requires every non-test file to export `__platforms` array** declaring supported platforms
27-
3. Parses import statements (ES6 imports, require(), dynamic imports) using TypeScript AST
28-
4. Validates that each import is compatible with the file's declared platforms
29-
5. Fails with exit code 1 if any violations are found or if `__platforms` export is missing
27+
3. **Validates platform values** by reading the Platform type definition from `platform_support.ts`
28+
4. Parses import statements (ES6 imports, require(), dynamic imports) using TypeScript AST
29+
5. **Validates import compatibility**: For each import, ensures the imported file supports ALL platforms that the importing file runs on
30+
6. Fails with exit code 1 if any violations are found, if `__platforms` export is missing, or if invalid platform values are used
31+
32+
**Import Rule**: When file A imports file B, file B must support ALL platforms that file A runs on.
33+
- Example: A universal file can only import from universal files (or files supporting all platforms)
34+
- Example: A browser file can import from universal files or any file that supports browser
3035

3136
**Note:** The validator can theoretically support file naming conventions (`.browser.ts`, etc.) in addition to `__platforms` exports, but currently enforces only the `__platforms` export. File naming is not validated and is considered redundant.
3237

scripts/validate-platform-isolation-ts.js

Lines changed: 158 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212
* - ALL source files (except tests) MUST export __platforms array
1313
* - Universal files use: export const __platforms = ['__universal__'];
1414
* - Platform-specific files use: export const __platforms = ['browser', 'node'];
15+
* - Valid platform values are dynamically read from Platform type in platform_support.ts
1516
*
1617
* Rules:
1718
* - Platform-specific files can only import from:
18-
* - Universal files (marked with '__universal__')
19+
* - Universal files (containing '__universal__' or all concrete platform values)
1920
* - Files supporting the same platforms
2021
* - External packages (node_modules)
2122
*
@@ -26,27 +27,92 @@ const fs = require('fs');
2627
const path = require('path');
2728
const ts = require('typescript');
2829

29-
const PLATFORMS = ['browser', 'node', 'react_native'];
3030
const LIB_DIR = path.join(__dirname, '..', 'lib');
31+
const PLATFORM_SUPPORT_FILE = path.join(LIB_DIR, 'platform_support.ts');
3132

3233
// Cache for __platforms exports
3334
const platformCache = new Map();
3435

35-
// Track files missing __platforms export
36-
const filesWithoutExport = [];
36+
// Valid platforms (loaded dynamically)
37+
let VALID_PLATFORMS = null;
38+
let ALL_CONCRETE_PLATFORMS = null;
39+
40+
/**
41+
* Extract valid platform values from Platform type definition
42+
* Parses: type Platform = 'browser' | 'node' | 'react_native' | '__universal__';
43+
*/
44+
function getValidPlatformsFromSource() {
45+
if (VALID_PLATFORMS !== null) {
46+
return VALID_PLATFORMS;
47+
}
48+
49+
try {
50+
const content = fs.readFileSync(PLATFORM_SUPPORT_FILE, 'utf-8');
51+
const sourceFile = ts.createSourceFile(
52+
PLATFORM_SUPPORT_FILE,
53+
content,
54+
ts.ScriptTarget.Latest,
55+
true
56+
);
57+
58+
function visit(node) {
59+
// Look for: export type Platform = ...
60+
if (ts.isTypeAliasDeclaration(node) &&
61+
ts.isIdentifier(node.name) &&
62+
node.name.text === 'Platform') {
63+
64+
const type = node.type;
65+
if (ts.isUnionTypeNode(type)) {
66+
const platforms = [];
67+
for (const member of type.types) {
68+
if (ts.isLiteralTypeNode(member) &&
69+
ts.isStringLiteral(member.literal)) {
70+
platforms.push(member.literal.text);
71+
}
72+
}
73+
VALID_PLATFORMS = platforms;
74+
ALL_CONCRETE_PLATFORMS = platforms.filter(p => p !== '__universal__');
75+
return;
76+
}
77+
}
78+
79+
ts.forEachChild(node, visit);
80+
}
81+
82+
visit(sourceFile);
83+
84+
if (!VALID_PLATFORMS) {
85+
throw new Error('Could not find Platform type definition in platform_support.ts');
86+
}
87+
88+
return VALID_PLATFORMS;
89+
} catch (error) {
90+
console.error('❌ Failed to read Platform type from platform_support.ts:', error.message);
91+
process.exit(1);
92+
}
93+
}
3794

3895
/**
39-
* Extracts the platform from a filename using naming convention
96+
* Gets a human-readable platform name
4097
*/
4198
function getPlatformFromFilename(filename) {
42-
for (const platform of PLATFORMS) {
99+
const validPlatforms = getValidPlatformsFromSource();
100+
const concretePlatforms = validPlatforms.filter(p => p !== '__universal__');
101+
102+
for (const platform of concretePlatforms) {
43103
if (filename.includes(`.${platform}.`)) {
44104
return platform;
45105
}
46106
}
47107
return null;
48108
}
49109

110+
// Track files missing __platforms export
111+
const filesWithoutExport = [];
112+
113+
// Track files with invalid platform values
114+
const filesWithInvalidPlatforms = [];
115+
50116
/**
51117
* Extracts __platforms array from AST
52118
*/
@@ -101,6 +167,35 @@ function extractSupportedPlatformsFromAST(sourceFile) {
101167
return platforms;
102168
}
103169

170+
/**
171+
* Validates that platform values are valid according to Platform type
172+
*/
173+
function validatePlatformValues(platforms, filePath) {
174+
if (!platforms || platforms.length === 0) {
175+
return { valid: false, invalidValues: [] };
176+
}
177+
178+
const validPlatforms = getValidPlatformsFromSource();
179+
const invalidValues = [];
180+
181+
for (const platform of platforms) {
182+
if (!validPlatforms.includes(platform)) {
183+
invalidValues.push(platform);
184+
}
185+
}
186+
187+
if (invalidValues.length > 0) {
188+
filesWithInvalidPlatforms.push({
189+
filePath,
190+
platforms,
191+
invalidValues
192+
});
193+
return { valid: false, invalidValues };
194+
}
195+
196+
return { valid: true, invalidValues: [] };
197+
}
198+
104199
/**
105200
* Gets the supported platforms for a file (with caching)
106201
* Returns:
@@ -132,6 +227,15 @@ function getSupportedPlatforms(filePath) {
132227
const supportedPlatforms = extractSupportedPlatformsFromAST(sourceFile);
133228

134229
if (supportedPlatforms && supportedPlatforms.length > 0) {
230+
// Validate platform values
231+
const validation = validatePlatformValues(supportedPlatforms, filePath);
232+
if (!validation.valid) {
233+
// Still cache it but it will be reported as error
234+
result = supportedPlatforms;
235+
platformCache.set(filePath, result);
236+
return result;
237+
}
238+
135239
result = supportedPlatforms;
136240
platformCache.set(filePath, result);
137241
return result;
@@ -176,41 +280,53 @@ function formatPlatforms(platforms) {
176280

177281
/**
178282
* Checks if platforms represent universal (all platforms)
283+
*
284+
* A file is universal if and only if:
285+
* 1. It contains '__universal__' in its platforms array
286+
*
287+
* Note: If array contains '__universal__' plus other values (e.g., ['__universal__', 'browser']),
288+
* it's still considered universal because __universal__ makes it available everywhere.
289+
*
290+
* Files that list all concrete platforms (e.g., ['browser', 'node', 'react_native']) are NOT
291+
* considered universal - they must explicitly declare '__universal__' to be universal.
179292
*/
180293
function isUniversal(platforms) {
181-
return Array.isArray(platforms) &&
182-
platforms.length === 1 &&
183-
platforms[0] === '__universal__';
294+
if (!Array.isArray(platforms) || platforms.length === 0) {
295+
return false;
296+
}
297+
298+
// ONLY if it explicitly declares __universal__, it's universal
299+
return platforms.includes('__universal__');
184300
}
185301

186302
/**
187303
* Checks if a platform is compatible with target platforms
188304
*
189305
* Rules:
190306
* - If either file is MISSING __platforms, not compatible
191-
* - Universal files are compatible with any file
192-
* - The import must support ALL platforms that the file supports
307+
* - Import must support ALL platforms that the importing file runs on
308+
* - Universal imports can be used by any file (they support all platforms)
309+
* - Platform-specific files can only import from universal or files supporting all their platforms
193310
*/
194311
function isPlatformCompatible(filePlatforms, importPlatforms) {
195312
// If either is missing platforms, not compatible
196313
if (filePlatforms === 'MISSING' || importPlatforms === 'MISSING') {
197314
return false;
198315
}
199316

200-
// If import is universal, always compatible
317+
// If import is universal, always compatible (universal supports all platforms)
201318
if (isUniversal(importPlatforms)) {
202319
return true;
203320
}
204321

205-
// If file is universal, import must be universal too
322+
// If file is universal but import is not, NOT compatible
323+
// (universal file runs everywhere, so imports must also run everywhere)
206324
if (isUniversal(filePlatforms)) {
207-
return isUniversal(importPlatforms);
325+
return false;
208326
}
209327

210-
// Otherwise, import must support ALL platforms that the file supports
211-
// filePlatforms is an array of platforms the file needs
212-
// importPlatforms is an array of platforms the import provides
213-
328+
// Otherwise, import must support ALL platforms that the file runs on
329+
// For each platform the file runs on, check if the import also supports it
214330
for (const platform of filePlatforms) {
215331
if (!importPlatforms.includes(platform)) {
216332
return false;
@@ -433,6 +549,10 @@ function main() {
433549

434550
const files = findSourceFiles(LIB_DIR);
435551

552+
// Load valid platforms first
553+
const validPlatforms = getValidPlatformsFromSource();
554+
console.log(`Valid platforms: ${validPlatforms.join(', ')}\n`);
555+
436556
// First pass: check for __platforms export
437557
console.log(`Found ${files.length} source files\n`);
438558
console.log('Checking for __platforms exports...\n');
@@ -465,6 +585,26 @@ function main() {
465585

466586
console.log('✅ All files have __platforms export\n');
467587

588+
// Report files with invalid platform values
589+
if (filesWithInvalidPlatforms.length > 0) {
590+
console.error(`❌ Found ${filesWithInvalidPlatforms.length} file(s) with invalid platform values:\n`);
591+
592+
for (const { filePath, platforms, invalidValues } of filesWithInvalidPlatforms) {
593+
const relativePath = path.relative(process.cwd(), filePath);
594+
console.error(` 📄 ${relativePath}`);
595+
console.error(` Declared: [${platforms.map(p => `'${p}'`).join(', ')}]`);
596+
console.error(` Invalid values: [${invalidValues.map(p => `'${p}'`).join(', ')}]`);
597+
}
598+
599+
console.error('\n');
600+
console.error(`Valid platform values: ${validPlatforms.map(p => `'${p}'`).join(', ')}`);
601+
console.error('See lib/platform_support.ts for Platform type definition.\n');
602+
603+
process.exit(1);
604+
}
605+
606+
console.log('✅ All __platforms arrays have valid values\n');
607+
468608
// Second pass: validate platform isolation
469609
console.log('Validating platform compatibility...\n');
470610

0 commit comments

Comments
 (0)