From cc0b8b902deadf546d49e09b2a0985e6f0caf892 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 18 Apr 2024 15:40:55 -0500 Subject: [PATCH 1/4] Don't allow use of the nullish coalescing operator in templates --- packages/nimble-components/src/.eslintrc.js | 9 +++++++++ .../nimble-components/src/rich-text/editor/template.ts | 2 +- .../src/table/components/cell/template.ts | 4 ++-- packages/nimble-components/src/table/template.ts | 4 ++-- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/nimble-components/src/.eslintrc.js b/packages/nimble-components/src/.eslintrc.js index 2cd445da13..03f2269028 100644 --- a/packages/nimble-components/src/.eslintrc.js +++ b/packages/nimble-components/src/.eslintrc.js @@ -120,6 +120,15 @@ module.exports = { '@typescript-eslint/indent': 'off' } }, + { + files: ['template.ts'], + rules: { + 'no-restricted-syntax': [ + 'error', + { selector: "LogicalExpression[operator='??']" } + ] + } + }, { // Instead of enums, this repo uses const objects and type unions which should live in types.ts files: ['types.ts'], diff --git a/packages/nimble-components/src/rich-text/editor/template.ts b/packages/nimble-components/src/rich-text/editor/template.ts index fb4618d2f5..65ab2428c5 100644 --- a/packages/nimble-components/src/rich-text/editor/template.ts +++ b/packages/nimble-components/src/rich-text/editor/template.ts @@ -117,7 +117,7 @@ export const template = html` ${ref('mentionListbox')} @mention-selected=${(x, c) => x.onMentionSelect(c.event as CustomEvent)} > - ${repeat(x => Array.from(x.activeMappingConfigs?.values() ?? []), html` + ${repeat(x => Array.from(x.activeMappingConfigs ? x.activeMappingConfigs.values() : []), html` <${listOptionTag} value="${x => x.mentionHref}">${x => x.displayName} `, { recycle: false })} diff --git a/packages/nimble-components/src/table/components/cell/template.ts b/packages/nimble-components/src/table/components/cell/template.ts index 20654d31e9..b3a30a4cdd 100644 --- a/packages/nimble-components/src/table/components/cell/template.ts +++ b/packages/nimble-components/src/table/components/cell/template.ts @@ -20,10 +20,10 @@ export const template = html` @toggle="${(x, c) => x.onActionMenuToggle(c.event as CustomEvent)}" @click="${(_, c) => c.event.stopPropagation()}" class="action-menu" - title="${x => x.actionMenuLabel ?? tableCellActionMenuLabel.getValueFor(x)}" + title="${x => (x.actionMenuLabel ? x.actionMenuLabel : tableCellActionMenuLabel.getValueFor(x))}" > <${iconThreeDotsLineTag} slot="start"> - ${x => x.actionMenuLabel ?? tableCellActionMenuLabel.getValueFor(x)} + ${x => (x.actionMenuLabel ? x.actionMenuLabel : tableCellActionMenuLabel.getValueFor(x))} `)} diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts index fd9ad33b96..0481c90995 100644 --- a/packages/nimble-components/src/table/template.ts +++ b/packages/nimble-components/src/table/template.ts @@ -42,7 +42,7 @@ export const template = html` --ni-private-table-header-container-margin-right: ${x => x.virtualizer.headerContainerMarginRight}px; --ni-private-table-scroll-height: ${x => x.virtualizer.scrollHeight}px; --ni-private-table-row-container-top: ${x => x.virtualizer.rowContainerYOffset}px; - --ni-private-table-row-grid-columns: ${x => x.rowGridColumns ?? ''}; + --ni-private-table-row-grid-columns: ${x => (x.rowGridColumns ? x.rowGridColumns : '')}; --ni-private-table-cursor-override: ${x => (x.layoutManager.isColumnBeingSized ? 'col-resize' : 'default')}; --ni-private-table-scrollable-min-width: ${x => x.tableScrollableMinWidth}px; --ni-private-glass-overlay-pointer-events: ${x => (x.layoutManager.isColumnBeingSized ? 'none' : 'default')}; @@ -60,7 +60,7 @@ export const template = html
` <${checkboxTag} ${ref('selectionCheckbox')} - class="${x => `selection-checkbox ${x.selectionMode ?? ''}`}" + class="${x => `selection-checkbox ${x.selectionMode ? x.selectionMode : ''}`}" @change="${(x, c) => x.onAllRowsSelectionChange(c.event as CustomEvent)}" title="${x => tableSelectAllLabel.getValueFor(x)}" aria-label="${x => tableSelectAllLabel.getValueFor(x)}" From fe100e60c7e65e88580852bd5adae975f510822c Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 18 Apr 2024 15:41:18 -0500 Subject: [PATCH 2/4] Change files --- ...le-components-0911f623-6d16-4c80-9b6d-0757470b2ca7.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@ni-nimble-components-0911f623-6d16-4c80-9b6d-0757470b2ca7.json diff --git a/change/@ni-nimble-components-0911f623-6d16-4c80-9b6d-0757470b2ca7.json b/change/@ni-nimble-components-0911f623-6d16-4c80-9b6d-0757470b2ca7.json new file mode 100644 index 0000000000..1dac19786e --- /dev/null +++ b/change/@ni-nimble-components-0911f623-6d16-4c80-9b6d-0757470b2ca7.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Don't allow use of the nullish coalescing operator in templates", + "packageName": "@ni/nimble-components", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} From 28a9472922f14b732c993c01392f81ae3200823c Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 19 Apr 2024 08:15:16 -0500 Subject: [PATCH 3/4] move rule to eslint-config-nimble --- packages/eslint-config-nimble/typescript.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/eslint-config-nimble/typescript.js b/packages/eslint-config-nimble/typescript.js index bb207768ce..aa6f8c31ec 100644 --- a/packages/eslint-config-nimble/typescript.js +++ b/packages/eslint-config-nimble/typescript.js @@ -118,6 +118,15 @@ module.exports = { '@typescript-eslint/indent': 'off' } }, + { + files: ['template.ts'], + rules: { + 'no-restricted-syntax': [ + 'error', + { selector: "LogicalExpression[operator='??']" } + ] + } + }, { // Instead of enums, this repo uses const objects and type unions which should live in types.ts files: ['types.ts'], From cd8b5d94c0de657be718ebe3a4716404d0b4a641 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 19 Apr 2024 11:21:20 -0500 Subject: [PATCH 4/4] Add comment --- packages/eslint-config-nimble/typescript.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/eslint-config-nimble/typescript.js b/packages/eslint-config-nimble/typescript.js index aa6f8c31ec..8141b53c64 100644 --- a/packages/eslint-config-nimble/typescript.js +++ b/packages/eslint-config-nimble/typescript.js @@ -121,6 +121,8 @@ module.exports = { { files: ['template.ts'], rules: { + // Using '??' in templates does not get flagged correctly by FAST as being a volatile binding. + // See https://github.com/ni/nimble/issues/1843 for more information. 'no-restricted-syntax': [ 'error', { selector: "LogicalExpression[operator='??']" }