Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/predicate-in-array-parse-catch-diagnostic-4266.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
---
'@object-ui/app-shell': patch
---

`predicate.ts`'s `in [...]` membership check now names an element it cannot
parse instead of silently discarding the whole set.

`path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
which JSON-parses it after normalising quotes. An element that is not a JSON
literal — a path, a bare identifier, a trailing comma — made that
`JSON.parse` throw, and the `catch` returned `[]` with nothing in the
console. `[].includes(anything)` is `false`, so the predicate silently read
FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
one bad element discarded every good literal sitting next to it too:
`data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
[data.a]` alone.

Same family as objectui#4049 (a silently wrong verdict, zero warning) and the
same ruling: **diagnose only, zero semantic change.** The `catch` still
returns `[]` — an `in` set that fails to parse is still, and remains, the
empty set; this evaluator does not gain the ability to resolve a path inside
`in [...]` (that stays outside the declared subset). All that changes is that
a dev-mode `console.warn` now names the predicate and, best-effort, the
element that broke the parse.

Fixes objectui#4266.
103 changes: 103 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -454,3 +454,106 @@ describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#404
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});

/* ── 8. a non-literal element inside `in [...]` is diagnosed (objectui#4266) ── */

/**
* objectui#4266. `parseLiteral`'s array branch JSON-parses the bracketed text
* after normalising quotes; an element that is not a JSON literal (a path, a
* bare identifier, a trailing comma) makes that `JSON.parse` throw, and the
* `catch` returned `[]` with nothing in the console — `in` reads FALSE FOR
* EVERY ROW, and because the parse is whole-set, one bad element discards the
* good literals sitting next to it too.
*
* Ruling on this card: **diagnose only, zero semantic change** — the same
* posture #4049 took for the right-hand-literal tail. The `catch` still
* returns `[]`; the verdicts are pinned IDENTICAL before and after (§8.3);
* all that changes is that the console stops being silent.
*/
describe('a non-literal element inside `in [...]` is diagnosed, not resolved (objectui#4266)', () => {
/* 8.1 — it fires, and it names the predicate and the culprit element */

it('`data.type in [data.a]` warns, naming the predicate and the unparseable element', () => {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain('data.type in [data.a]');
expect(warnings()).toContain('`data.a`');
});

it('a bare (unresolvable-shaped) identifier element is named too', () => {
expect(evaluatePredicate("data.type in ['text', foo]", scope({ type: 'text' }))).toBe(false);
expect(warnings()).toContain('`foo`');
});

it('a trailing comma with otherwise-literal elements still warns, falling back to the whole set', () => {
// No single element is at fault here — every element parses fine on its
// own, so `findUnparseableSetElement` finds none, and the warning names
// the whole broken set instead of guessing at (and misnaming) a culprit.
expect(evaluatePredicate("data.type in ['a','b',]", scope({ type: 'a' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
expect(warnings()).toContain("data.type in ['a','b',]");
expect(warnings()).not.toContain('containing `');
});

/* 8.2 — one bad element still discards the good literals beside it, and the
warning names the ONE that actually broke the parse, not the whole set
blindly */

it('one non-literal element collapses the WHOLE set, including the good literals next to it', () => {
// Both sides hold 'text' — a working `in` would be true. It is false,
// matching the pre-fix whole-set collapse; only the console changes.
expect(evaluatePredicate("data.type in ['text', data.a]", scope({ type: 'text', a: 'text' }))).toBe(
false,
);
expect(warnings()).toContain('`data.a`');
// The warning names the culprit, not the innocent literal beside it.
expect(warnings()).not.toContain("containing `'text'`");
});

/* 8.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.type in [data.a]', { type: 'text', a: 'text' }, false], // would be true if paths resolved
["data.type in ['text', data.a]", { type: 'text', a: 'text' }, false], // good literal discarded too
["data.type in [data.a,]", { type: 'text', a: 'text' }, false],
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
});

/* 8.4 — controls: literal-only `in` sets are completely unaffected */

it.each([
["data.type in ['text','textarea']", { type: 'text' }, true],
["data.type in ['number','currency']", { type: 'text' }, false],
["data.type in ['a', 'b', 'c']", {}, false],
])('%s over %j → %s, silently (literal path unperturbed)', (expr, row, expected) => {
expect(evaluatePredicate(expr, scope(row as Record<string, unknown>))).toBe(expected);
expect(warn).not.toHaveBeenCalled();
});

/* 8.5 — warn-once discipline, same bar as #6936 and #4049 */

it('warns ONCE per (predicate, set) pair, not once per evaluation', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same broken set gets its own warning', () => {
evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'x' }));
evaluatePredicate('data.kind in [data.a]', scope({ kind: 'text', a: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

/* 8.6 — dev-mode only */

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.type in [data.a]', scope({ type: 'text', a: 'text' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});
});
102 changes: 102 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,6 +104,43 @@
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*
* ## An unparseable element inside `in [...]` also fails silently (objectui#4266)
*
* `path in [...]` hands the bracketed text to `parseLiteral`'s array branch,
* which JSON-parses it after normalising quotes. An element that is not a JSON
* literal — a path, a bare identifier, a trailing comma — makes that
* `JSON.parse` throw, and the `catch` returned `[]` with nothing in the
* console. `[].includes(anything)` is `false`, so the predicate silently reads
* FALSE FOR EVERY ROW, and — because the parse is whole-set, not per-element —
* one bad element discards every good literal sitting next to it too:
* `data.type in ['text', data.a]` collapsed exactly as hard as `data.type in
* [data.a]` alone.
*
* Same family as objectui#4049 (silently wrong verdict, zero warning) and the
* same ruling: **diagnose only, zero semantic change.** The `catch` still
* returns `[]` — an `in` set that fails to parse is still, and remains, the
* empty set — this file does not gain the ability to resolve a path inside
* `in [...]` (that stays outside the declared subset; #4049 already draws
* that boundary for the right side of `==`/`!=` and it is not reopened here).
* All that changes is that the console names the predicate and, best-effort,
* the element that broke the parse, instead of staying silent. Verdicts are
* pinned identical before and after in `predicate.test.ts` §8.3, the same
* shape as #4049's §7.3.
*
* A louder option — making the whole predicate throw so the top-level
* fail-open in {@link evaluatePredicate} turns it `true`, mirroring
* objectstack#6936's unresolved-path ruling — was considered and rejected:
* #6936's `true` verdict corrects a fail-CLOSED bug (a hidden field is worse
* than a shown one), but here the existing verdict (`false`, i.e. hidden) is
* not a bug — it is the documented behaviour for a set this evaluator cannot
* parse, same as `#4049`'s tail returning the right-hand text verbatim
* instead of resolving it. Flipping it to fail-open `true` would be a
* semantic change with no ruling behind it, and would make an authoring
* mistake (a stray non-literal element) MORE visible than a correctly
* authored predicate that legitimately evaluates false — exactly backwards.
*
* This diagnostic retires with the file at ROADMAP M9, same as #4049's.
*/

export function evaluatePredicate(
Expand DownExpand Up@@ -151,10 +188,20 @@ const warnedUnresolvedPaths = new Set<string>();
*/
const warnedPathShapedLiterals = new Set<string>();

/**
* The same warn-once discipline for the `in`-array parse-failure diagnostic
* (objectui#4266), keyed on (raw set text, predicate) for the same reason as
* the two Sets above: keying on the set text alone would report the first
* predicate carrying it and stay silent about a sibling predicate that
* happens to spell the same broken set.
*/
const warnedUnparseableInSets = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
warnedUnparseableInSets.clear();
}

const isDev = (): boolean =>
Expand DownExpand Up@@ -210,6 +257,56 @@ function warnPathShapedLiteral(text: string, source: string): void {
);
}

/**
* Best-effort identification of WHICH element inside an unparseable `in [...]`
* set actually broke the parse — for the warning text only, never to change
* the return value. Splits on top-level commas (reusing `splitTopLevel`,
* which already tracks bracket depth and quotes for the `&&`/`||` splitters
* above) and re-runs the exact same quote-normalising `JSON.parse` the array
* branch itself uses, one element at a time, so the reported culprit is
* judged by literally the same rule that judged the whole set. Returns `null`
* when every individual element parses fine on its own (e.g. a stray trailing
* comma broke the whole-string parse but no single element is at fault) —
* the warning then falls back to naming the whole set.
*/
function findUnparseableSetElement(raw: string): string | null {
const inner = raw.slice(1, -1);
if (!inner.trim()) return null;
for (const part of splitTopLevel(inner, ',')) {
const el = part.trim();
if (!el) continue;
try {
const json = el.replace(/'([^']*)'/g, (_, innerStr) => JSON.stringify(innerStr));
JSON.parse(json);
} catch {
return el;
}
}
return null;
}

function warnUnparseableInSet(raw: string, source: string): void {
if (!isDev()) return;
const memo = `${raw}::${source}`;
if (warnedUnparseableInSets.has(memo)) return;
warnedUnparseableInSets.add(memo);
const element = findUnparseableSetElement(raw);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` has an \`in\` set \`${raw}\`` +
(element != null
? ` containing \`${element}\`, which is not a literal this evaluator can parse — `
: ', which this evaluator could not parse — ') +
'the WHOLE set was treated as EMPTY, so the predicate is FALSE for every row (not just the ' +
'element that failed — one bad element discards the good literals next to it too). This ' +
"evaluator only supports literal elements inside `in [...]` (supported subset: `path in " +
"['a','b']`); it does not resolve paths there — paths resolve only on the LEFT of an operator " +
'(objectui#4049). If you meant a literal, quote it. If you meant to test membership against ' +
"another field, that is outside this evaluator's subset: it is an interim stand-in for " +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are validated ' +
'at publish time (objectstack#7010). objectui#4266.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
Expand DownExpand Up@@ -334,6 +431,11 @@ function parseLiteral(raw: string, source: string): unknown {
const json = s.replace(/'([^']*)'/g, (_, inner) => JSON.stringify(inner));
return JSON.parse(json);
} catch {
// Whole-set parse failure (a non-literal element, a trailing comma, …).
// Diagnose only (objectui#4266) — the verdict is UNTOUCHED: the set is
// still `[]`, `in` is still false for every row. See the header section
// "An unparseable element inside `in [...]` also fails silently".
warnUnparseableInSet(s, source);
return [];
}
}
Expand Down
Loading