From e0ac261bd567de847978525d6a9c96fd2f29e394 Mon Sep 17 00:00:00 2001 From: Alan Treadway Date: Mon, 12 Jan 2026 09:33:48 +0000 Subject: [PATCH 1/7] git subrepo push external/ag-shared (#12857) subrepo: subdir: "external/ag-shared" merged: "fbd90b4570d" upstream: origin: "https://github.com/ag-grid/ag-shared.git" branch: "latest" commit: "fbd90b4570d" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "70badad488" --- external/ag-shared/.gitrepo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/external/ag-shared/.gitrepo b/external/ag-shared/.gitrepo index f15d50dec30..a822c587cae 100644 --- a/external/ag-shared/.gitrepo +++ b/external/ag-shared/.gitrepo @@ -6,7 +6,7 @@ [subrepo] remote = https://github.com/ag-grid/ag-shared.git branch = latest - commit = 21cf936f7e005a9650d0da35c0b16ffd76e7451e - parent = ad6a693623ad2fc08c9634ac21787f470335bb4b + commit = fbd90b4570de9da8b042534e8f5127a4cac4ee17 + parent = b9dd812de3f958b32ebc09e02aa014041afbfe4d method = rebase cmdver = 0.4.9 From c241c375b5fb7bcec86a4112e90de816ab6e71d9 Mon Sep 17 00:00:00 2001 From: Stephen Cooper Date: Mon, 12 Jan 2026 10:01:19 +0000 Subject: [PATCH 2/7] Test module size reporting (#12849) * Test module size reporting * Remove total as not a meaningful number * Break find modules to validate test * fix formatting * Only re-run latest if it has been updated * Fix code and small tweaks * Enable manual run against any PR on demand * Fix lint and drop limit as it is not blocking * add shard count variable and clean up report * add release comparison check * disable failing CI --- .github/actions/module-size-test/action.yml | 57 ++++ .github/workflows/module-size-comparison.yml | 322 +++++++++++++++++++ .github/workflows/module-size-vs-release.yml | 283 ++++++++++++++++ scripts/ci/compare-module-sizes.mjs | 231 +++++++++++++ scripts/ci/merge-module-size-results.mjs | 69 ++++ testing/module-size/moduleValidator.ts | 3 +- 6 files changed, 964 insertions(+), 1 deletion(-) create mode 100644 .github/actions/module-size-test/action.yml create mode 100644 .github/workflows/module-size-comparison.yml create mode 100644 .github/workflows/module-size-vs-release.yml create mode 100644 scripts/ci/compare-module-sizes.mjs create mode 100644 scripts/ci/merge-module-size-results.mjs diff --git a/.github/actions/module-size-test/action.yml b/.github/actions/module-size-test/action.yml new file mode 100644 index 00000000000..6b6275e5d03 --- /dev/null +++ b/.github/actions/module-size-test/action.yml @@ -0,0 +1,57 @@ +name: Module Size Test +description: Build and run module size tests for a given git ref + +inputs: + ref: + description: 'Git ref to checkout (branch, tag, or SHA)' + required: true + shard: + description: 'Current shard number' + required: true + total-shards: + description: 'Total number of shards' + required: true + artifact-prefix: + description: 'Prefix for the uploaded artifact name' + required: true + artifact-suffix: + description: 'Optional suffix for artifact name (e.g., PR number)' + required: false + default: '' + base-ref: + description: 'Base ref for nx setup (defaults to input ref)' + required: false + +runs: + using: composite + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + ref: ${{ inputs.ref }} + fetch-depth: 1 + + - name: Setup + uses: ./.github/actions/setup-nx + with: + cache_mode: ro + base_ref: ${{ inputs.base-ref || inputs.ref }} + + - name: Build dependencies + shell: bash + run: | + yarn nx run-many -t build --projects=ag-grid-community,ag-grid-enterprise,ag-grid-react + + - name: Run module size tests + shell: bash + working-directory: testing/module-size + run: | + npm run module-combinations -- --shard=${{ inputs.shard }}/${{ inputs.total-shards }} + mv module-size-results.json module-size-results-shard-${{ inputs.shard }}.json + + - name: Upload shard results + uses: actions/upload-artifact@v4 + with: + name: ${{ inputs.artifact-prefix }}-shard-${{ inputs.shard }}${{ inputs.artifact-suffix && format('-{0}', inputs.artifact-suffix) || '' }} + path: testing/module-size/module-size-results-shard-${{ inputs.shard }}.json + retention-days: 1 diff --git a/.github/workflows/module-size-comparison.yml b/.github/workflows/module-size-comparison.yml new file mode 100644 index 00000000000..cfc0b2fe230 --- /dev/null +++ b/.github/workflows/module-size-comparison.yml @@ -0,0 +1,322 @@ +name: Module Size Comparison + +on: + pull_request: + branches: + - 'latest' + types: [opened, synchronize, reopened, ready_for_review] + workflow_dispatch: + inputs: + pr_number: + description: 'PR number to compare (for manual runs)' + type: number + required: true + +env: + NX_NO_CLOUD: true + NX_BRANCH: ${{ github.ref }} + SHARP_IGNORE_GLOBAL_LIBVIPS: true + YARN_REGISTRY: "https://registry.ag-grid.com" + CI: true + SHARD_COUNT: 3 + +concurrency: + group: module-size-${{ github.event.pull_request.number || inputs.pr_number || github.run_id }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: write + +jobs: + prepare: + name: Prepare Context + runs-on: ubuntu-latest + outputs: + pr-number: ${{ steps.resolve.outputs.pr-number }} + head-sha: ${{ steps.resolve.outputs.head-sha }} + is-draft: ${{ steps.resolve.outputs.is-draft }} + shard-matrix: ${{ steps.resolve.outputs.shard-matrix }} + steps: + - name: Resolve PR details + id: resolve + uses: actions/github-script@v7 + env: + SHARD_COUNT: ${{ env.SHARD_COUNT }} + with: + script: | + const shardCount = parseInt(process.env.SHARD_COUNT, 10); + const shards = Array.from({ length: shardCount }, (_, i) => i + 1); + core.setOutput('shard-matrix', JSON.stringify(shards)); + + const eventName = context.eventName; + if (eventName === 'pull_request') { + const pr = context.payload.pull_request; + core.setOutput('pr-number', pr.number.toString()); + core.setOutput('head-sha', pr.head.sha); + core.setOutput('is-draft', pr.draft ? 'true' : 'false'); + return; + } + + const manualPrNumber = context.payload?.inputs?.pr_number; + if (!manualPrNumber) { + core.setFailed('workflow_dispatch requires pr_number input.'); + return; + } + + const pull_number = Number(manualPrNumber); + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number, + }); + + if (!pr) { + core.setFailed(`Could not load pull request ${pull_number}.`); + return; + } + + core.setOutput('pr-number', pr.number.toString()); + core.setOutput('head-sha', pr.head.sha); + core.setOutput('is-draft', pr.draft ? 'true' : 'false'); + + # Check for cached base results + check-base-cache: + name: Check Base Cache + runs-on: ubuntu-latest + needs: prepare + if: needs.prepare.outputs.is-draft != 'true' + outputs: + cache-hit: ${{ steps.cache-check.outputs.cache-hit }} + latest-sha: ${{ steps.get-sha.outputs.sha }} + steps: + - name: Get latest branch SHA + id: get-sha + run: | + SHA=$(git ls-remote https://github.com/${{ github.repository }} refs/heads/latest | cut -f1) + echo "sha=${SHA}" >> $GITHUB_OUTPUT + echo "Latest branch SHA: ${SHA}" + + - name: Check cache for base results + id: cache-check + uses: actions/cache/restore@v4 + with: + path: ./base-results.json + key: module-size-base-${{ steps.get-sha.outputs.sha }} + lookup-only: true + + # Build and run module size tests on the PR branch (sharded) + module-size-pr: + name: Module Size PR (${{ matrix.shard }}/${{ strategy.job-total }}) + runs-on: ubuntu-latest + needs: prepare + if: needs.prepare.outputs.is-draft != 'true' + strategy: + matrix: + shard: ${{ fromJSON(needs.prepare.outputs.shard-matrix) }} + fail-fast: false + steps: + - name: Checkout for actions + uses: actions/checkout@v4 + with: + sparse-checkout: .github/actions + ref: ${{ needs.prepare.outputs.head-sha }} + + - name: Run module size tests + uses: ./.github/actions/module-size-test + with: + ref: ${{ needs.prepare.outputs.head-sha }} + shard: ${{ matrix.shard }} + total-shards: ${{ strategy.job-total }} + artifact-prefix: module-size-pr + artifact-suffix: ${{ needs.prepare.outputs.pr-number }} + + # Build and run module size tests on the latest branch (sharded) - only if cache miss + module-size-base: + name: Module Size Base (${{ matrix.shard }}/${{ strategy.job-total }}) + runs-on: ubuntu-latest + needs: [prepare, check-base-cache] + if: needs.prepare.outputs.is-draft != 'true' && needs.check-base-cache.outputs.cache-hit != 'true' + strategy: + matrix: + shard: ${{ fromJSON(needs.prepare.outputs.shard-matrix) }} + fail-fast: false + steps: + - name: Checkout for actions + uses: actions/checkout@v4 + with: + sparse-checkout: .github/actions + ref: ${{ needs.prepare.outputs.head-sha }} + + - name: Run module size tests + uses: ./.github/actions/module-size-test + with: + ref: latest + shard: ${{ matrix.shard }} + total-shards: ${{ strategy.job-total }} + artifact-prefix: module-size-base + artifact-suffix: ${{ needs.prepare.outputs.pr-number }} + base-ref: latest + + # Merge base shards and save to cache (only when we had to build) + cache-base-results: + name: Cache Base Results + runs-on: ubuntu-latest + needs: [prepare, check-base-cache, module-size-base] + if: always() && needs.module-size-base.result == 'success' + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + sparse-checkout: | + scripts/ci/merge-module-size-results.mjs + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Download all base shard results + uses: actions/download-artifact@v4 + with: + pattern: module-size-base-shard-*-${{ needs.prepare.outputs.pr-number }} + path: ./base-shards + merge-multiple: true + + - name: Merge shard results + run: | + node scripts/ci/merge-module-size-results.mjs ./base-shards ./base-results.json + + - name: Save to cache + uses: actions/cache/save@v4 + with: + path: ./base-results.json + key: module-size-base-${{ needs.check-base-cache.outputs.latest-sha }} + + # Compare results and post PR comment + compare-and-report: + name: Compare & Report + runs-on: ubuntu-latest + needs: [prepare, check-base-cache, module-size-pr, module-size-base, cache-base-results] + # Run if PR tests succeeded AND either cache hit OR base tests succeeded + if: | + always() && + needs.module-size-pr.result == 'success' && + (needs.check-base-cache.outputs.cache-hit == 'true' || needs.module-size-base.result == 'success') + env: + PR_NUMBER: ${{ needs.prepare.outputs.pr-number }} + CACHE_HIT: ${{ needs.check-base-cache.outputs.cache-hit }} + LATEST_SHA: ${{ needs.check-base-cache.outputs.latest-sha }} + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + sparse-checkout: | + scripts/ci/compare-module-sizes.mjs + scripts/ci/merge-module-size-results.mjs + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Download all PR shard results + uses: actions/download-artifact@v4 + with: + pattern: module-size-pr-shard-*-${{ env.PR_NUMBER }} + path: ./pr-shards + merge-multiple: true + + - name: Merge PR shard results + run: | + node scripts/ci/merge-module-size-results.mjs ./pr-shards ./pr-results.json + + # If cache hit, restore from cache + - name: Restore base results from cache + if: env.CACHE_HIT == 'true' + uses: actions/cache/restore@v4 + with: + path: ./base-results.json + key: module-size-base-${{ env.LATEST_SHA }} + fail-on-cache-miss: true + + # If cache miss, download artifacts and merge + - name: Download all base shard results + if: env.CACHE_HIT != 'true' + uses: actions/download-artifact@v4 + with: + pattern: module-size-base-shard-*-${{ env.PR_NUMBER }} + path: ./base-shards + merge-multiple: true + + - name: Merge base shard results + if: env.CACHE_HIT != 'true' + run: | + node scripts/ci/merge-module-size-results.mjs ./base-shards ./base-results.json + + - name: Compare module sizes + id: compare + run: | + node scripts/ci/compare-module-sizes.mjs \ + ./base-results.json \ + ./pr-results.json \ + ./comparison-report.md + + - name: Find existing comment + id: find-comment + uses: actions/github-script@v7 + with: + result-encoding: string + script: | + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: parseInt(process.env.PR_NUMBER, 10), + }); + + const botComment = comments.find(comment => + comment.user.type === 'Bot' && + comment.body.includes('## Module Size Comparison') + ); + + return botComment ? botComment.id : ''; + + - name: Post or update comment + uses: actions/github-script@v7 + env: + COMMENT_ID: ${{ steps.find-comment.outputs.result }} + with: + script: | + const fs = require('fs'); + const report = fs.readFileSync('./comparison-report.md', 'utf8'); + const owner = context.repo.owner; + const repo = context.repo.repo; + const issue_number = parseInt(process.env.PR_NUMBER, 10); + const comment_id = process.env.COMMENT_ID; + + const body = report + '\n\n---\n*Updated: ' + new Date().toISOString() + '*'; + + if (comment_id) { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: parseInt(comment_id, 10), + body + }); + console.log(`Updated comment ${comment_id}`); + } else { + const { data: comment } = await github.rest.issues.createComment({ + owner, + repo, + issue_number, + body + }); + console.log(`Created comment ${comment.id}`); + } + + - name: Upload comparison report + uses: actions/upload-artifact@v4 + with: + name: module-size-comparison-${{ env.PR_NUMBER }} + path: ./comparison-report.md + retention-days: 30 diff --git a/.github/workflows/module-size-vs-release.yml b/.github/workflows/module-size-vs-release.yml new file mode 100644 index 00000000000..1da22203212 --- /dev/null +++ b/.github/workflows/module-size-vs-release.yml @@ -0,0 +1,283 @@ +name: Module Size vs Release + +on: + workflow_dispatch: + inputs: + release_tag: + description: 'Release tag to compare against (e.g., b35.1.0)' + type: string + required: true + schedule: + # Run weekly on Monday at 9am UTC + - cron: '0 9 * * 1' + +env: + NX_NO_CLOUD: true + NX_BRANCH: ${{ github.ref }} + SHARP_IGNORE_GLOBAL_LIBVIPS: true + YARN_REGISTRY: "https://registry.ag-grid.com" + CI: true + SHARD_COUNT: 3 + +concurrency: + group: module-size-release-${{ inputs.release_tag || 'scheduled' }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + prepare: + name: Prepare Context + runs-on: ubuntu-latest + outputs: + release-tag: ${{ steps.resolve.outputs.release-tag }} + shard-matrix: ${{ steps.resolve.outputs.shard-matrix }} + steps: + - name: Resolve release tag + id: resolve + uses: actions/github-script@v7 + env: + SHARD_COUNT: ${{ env.SHARD_COUNT }} + INPUT_TAG: ${{ inputs.release_tag }} + with: + script: | + const shardCount = parseInt(process.env.SHARD_COUNT, 10); + const shards = Array.from({ length: shardCount }, (_, i) => i + 1); + core.setOutput('shard-matrix', JSON.stringify(shards)); + + let releaseTag = process.env.INPUT_TAG; + + // If no tag provided (scheduled run), find the latest release tag + if (!releaseTag) { + const { data: releases } = await github.rest.repos.listReleases({ + owner: context.repo.owner, + repo: context.repo.repo, + per_page: 10 + }); + + // Find the latest non-prerelease tag matching bXX.X.X pattern + const release = releases.find(r => !r.prerelease && /^b\d+\.\d+\.\d+$/.test(r.tag_name)); + if (!release) { + core.setFailed('Could not find a valid release tag'); + return; + } + releaseTag = release.tag_name; + console.log(`Using latest release tag: ${releaseTag}`); + } + + core.setOutput('release-tag', releaseTag); + + # Check for cached release results (releases are immutable, so cache indefinitely) + check-release-cache: + name: Check Release Cache + runs-on: ubuntu-latest + needs: prepare + outputs: + cache-hit: ${{ steps.cache-check.outputs.cache-hit }} + steps: + - name: Check cache for release results + id: cache-check + uses: actions/cache/restore@v4 + with: + path: ./release-results.json + key: module-size-release-${{ needs.prepare.outputs.release-tag }} + lookup-only: true + + # Build and run module size tests on latest branch (sharded) + module-size-latest: + name: Module Size Latest (${{ matrix.shard }}/${{ strategy.job-total }}) + runs-on: ubuntu-latest + needs: prepare + strategy: + matrix: + shard: ${{ fromJSON(needs.prepare.outputs.shard-matrix) }} + fail-fast: false + steps: + - name: Checkout for actions + uses: actions/checkout@v4 + with: + sparse-checkout: .github/actions + ref: latest + + - name: Run module size tests + uses: ./.github/actions/module-size-test + with: + ref: latest + shard: ${{ matrix.shard }} + total-shards: ${{ strategy.job-total }} + artifact-prefix: module-size-latest + base-ref: latest + + # Build and run module size tests on release tag (sharded) - only if cache miss + module-size-release: + name: Module Size Release (${{ matrix.shard }}/${{ strategy.job-total }}) + runs-on: ubuntu-latest + needs: [prepare, check-release-cache] + if: needs.check-release-cache.outputs.cache-hit != 'true' + strategy: + matrix: + shard: ${{ fromJSON(needs.prepare.outputs.shard-matrix) }} + fail-fast: false + steps: + - name: Checkout for actions + uses: actions/checkout@v4 + with: + sparse-checkout: .github/actions + ref: latest + + - name: Run module size tests + uses: ./.github/actions/module-size-test + with: + ref: ${{ needs.prepare.outputs.release-tag }} + shard: ${{ matrix.shard }} + total-shards: ${{ strategy.job-total }} + artifact-prefix: module-size-release + base-ref: ${{ needs.prepare.outputs.release-tag }} + + # Cache release results (only when we had to build) + cache-release-results: + name: Cache Release Results + runs-on: ubuntu-latest + needs: [prepare, check-release-cache, module-size-release] + if: always() && needs.module-size-release.result == 'success' + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + sparse-checkout: | + scripts/ci/merge-module-size-results.mjs + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Download all release shard results + uses: actions/download-artifact@v4 + with: + pattern: module-size-release-shard-* + path: ./release-shards + merge-multiple: true + + - name: Merge shard results + run: | + node scripts/ci/merge-module-size-results.mjs ./release-shards ./release-results.json + + - name: Save to cache + uses: actions/cache/save@v4 + with: + path: ./release-results.json + key: module-size-release-${{ needs.prepare.outputs.release-tag }} + + # Compare results and create/update GitHub issue + compare-and-report: + name: Compare & Report + runs-on: ubuntu-latest + needs: [prepare, check-release-cache, module-size-latest, module-size-release, cache-release-results] + if: | + always() && + needs.module-size-latest.result == 'success' && + (needs.check-release-cache.outputs.cache-hit == 'true' || needs.module-size-release.result == 'success') + env: + RELEASE_TAG: ${{ needs.prepare.outputs.release-tag }} + CACHE_HIT: ${{ needs.check-release-cache.outputs.cache-hit }} + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + sparse-checkout: | + scripts/ci/compare-module-sizes.mjs + scripts/ci/merge-module-size-results.mjs + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Download all latest shard results + uses: actions/download-artifact@v4 + with: + pattern: module-size-latest-shard-* + path: ./latest-shards + merge-multiple: true + + - name: Merge latest shard results + run: | + node scripts/ci/merge-module-size-results.mjs ./latest-shards ./latest-results.json + + # If cache hit, restore from cache + - name: Restore release results from cache + if: env.CACHE_HIT == 'true' + uses: actions/cache/restore@v4 + with: + path: ./release-results.json + key: module-size-release-${{ env.RELEASE_TAG }} + fail-on-cache-miss: true + + # If cache miss, download artifacts and merge + - name: Download all release shard results + if: env.CACHE_HIT != 'true' + uses: actions/download-artifact@v4 + with: + pattern: module-size-release-shard-* + path: ./release-shards + merge-multiple: true + + - name: Merge release shard results + if: env.CACHE_HIT != 'true' + run: | + node scripts/ci/merge-module-size-results.mjs ./release-shards ./release-results.json + + - name: Compare module sizes + run: | + node scripts/ci/compare-module-sizes.mjs \ + ./release-results.json \ + ./latest-results.json \ + ./comparison-report.md + + - name: Output report to summary + run: | + echo "# Module Size Comparison: \`latest\` vs \`${{ env.RELEASE_TAG }}\`" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + cat ./comparison-report.md >> $GITHUB_STEP_SUMMARY + + - name: Upload comparison report + uses: actions/upload-artifact@v4 + with: + name: module-size-vs-release-${{ env.RELEASE_TAG }} + path: ./comparison-report.md + retention-days: 90 + + # Report failure + report-failure: + name: Report Failure + runs-on: ubuntu-latest + needs: [prepare, check-release-cache, module-size-latest, module-size-release, compare-and-report] + if: | + always() && + needs.compare-and-report.result == 'skipped' + env: + RELEASE_TAG: ${{ needs.prepare.outputs.release-tag }} + LATEST_RESULT: ${{ needs.module-size-latest.result }} + RELEASE_RESULT: ${{ needs.module-size-release.result }} + CACHE_HIT: ${{ needs.check-release-cache.outputs.cache-hit }} + steps: + - name: Report failure + run: | + if [[ "$LATEST_RESULT" != "success" ]]; then + REASON="Latest branch module size tests failed ($LATEST_RESULT)" + elif [[ "$CACHE_HIT" != "true" && "$RELEASE_RESULT" != "success" ]]; then + REASON="Release $RELEASE_TAG module size tests failed ($RELEASE_RESULT)" + else + REASON="Unknown failure" + fi + + echo "# Module Size Comparison Failed" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Release tag:** $RELEASE_TAG" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Reason:** $REASON" >> $GITHUB_STEP_SUMMARY + + echo "::error::Module size comparison failed: $REASON" + exit 1 diff --git a/scripts/ci/compare-module-sizes.mjs b/scripts/ci/compare-module-sizes.mjs new file mode 100644 index 00000000000..8085aeacae5 --- /dev/null +++ b/scripts/ci/compare-module-sizes.mjs @@ -0,0 +1,231 @@ +#!/usr/bin/env node +import fs from 'node:fs'; + +const THRESHOLD_KB = 1; // Report modules with size changes + +/** + * Compare two module-size-results.json files and generate a diff report + * + * Usage: node compare-module-sizes.mjs + */ + +const baseFile = process.argv[2]; +const prFile = process.argv[3]; +const outputFile = process.argv[4] || './module-size-comparison.md'; + +if (!baseFile || !prFile) { + console.error('Usage: node compare-module-sizes.mjs [output-file]'); + process.exit(1); +} + +function loadResults(filePath) { + try { + const content = fs.readFileSync(filePath, 'utf8'); + return JSON.parse(content); + } catch (error) { + console.error(`Failed to load ${filePath}:`, error.message); + process.exit(1); + } +} + +function getModuleKey(modules) { + return modules.length === 0 ? '__base__' : modules.sort().join('+'); +} + +function formatSize(size) { + return size.toFixed(2); +} + +function formatDiff(diff) { + const sign = diff >= 0 ? '+' : ''; + return `${sign}${diff.toFixed(2)}`; +} + +function getChangeEmoji(diff) { + if (diff > 0) { + return '📈'; + } + if (diff < 0) { + return '📉'; + } + return '➖'; +} + +const baseResults = loadResults(baseFile); +const prResults = loadResults(prFile); + +// Create maps for easy lookup +const baseMap = new Map(); +baseResults.forEach((result) => { + baseMap.set(getModuleKey(result.modules), result); +}); + +const prMap = new Map(); +prResults.forEach((result) => { + prMap.set(getModuleKey(result.modules), result); +}); + +// Calculate diffs +const diffs = []; +const allKeys = new Set([...baseMap.keys(), ...prMap.keys()]); + +for (const key of allKeys) { + const base = baseMap.get(key); + const pr = prMap.get(key); + + if (base && pr) { + const selfSizeDiff = pr.selfSize - base.selfSize; + const fileSizeDiff = pr.fileSize - base.fileSize; + const gzipSizeDiff = pr.gzipSize - base.gzipSize; + + diffs.push({ + modules: pr.modules, + key, + baseSelfSize: base.selfSize, + prSelfSize: pr.selfSize, + selfSizeDiff, + baseFileSize: base.fileSize, + prFileSize: pr.fileSize, + fileSizeDiff, + baseGzipSize: base.gzipSize, + prGzipSize: pr.gzipSize, + gzipSizeDiff, + isNew: false, + isRemoved: false, + }); + } else if (pr && !base) { + diffs.push({ + modules: pr.modules, + key, + baseSelfSize: 0, + prSelfSize: pr.selfSize, + selfSizeDiff: pr.selfSize, + baseFileSize: 0, + prFileSize: pr.fileSize, + fileSizeDiff: pr.fileSize, + baseGzipSize: 0, + prGzipSize: pr.gzipSize, + gzipSizeDiff: pr.gzipSize, + isNew: true, + isRemoved: false, + }); + } else if (base && !pr) { + diffs.push({ + modules: base.modules, + key, + baseSelfSize: base.selfSize, + prSelfSize: 0, + selfSizeDiff: -base.selfSize, + baseFileSize: base.fileSize, + prFileSize: 0, + fileSizeDiff: -base.fileSize, + baseGzipSize: base.gzipSize, + prGzipSize: 0, + gzipSizeDiff: -base.gzipSize, + isNew: false, + isRemoved: true, + }); + } +} + +// Sort by absolute diff size (largest first) +diffs.sort((a, b) => Math.abs(b.selfSizeDiff) - Math.abs(a.selfSizeDiff)); + +// Find extremes +const maxIncrease = diffs.reduce((max, d) => (d.selfSizeDiff > max.selfSizeDiff ? d : max), diffs[0]); +const maxDecrease = diffs.reduce((min, d) => (d.selfSizeDiff < min.selfSizeDiff ? d : min), diffs[0]); + +// Filter significant changes +const significantChanges = diffs.filter((d) => Math.abs(d.selfSizeDiff) >= THRESHOLD_KB); + +// Generate markdown report +let report = ''; + +// Header +report += '## Module Size Comparison\n\n'; + +// Extremes section +report += '### Extreme Values\n\n'; + +if (maxIncrease && maxIncrease.selfSizeDiff > 0) { + const moduleName = maxIncrease.modules.length === 0 ? 'Base (no modules)' : maxIncrease.modules.join(', '); + report += `📈 **Largest Increase:** ${moduleName}\n`; + report += ` - Self Size: ${formatSize(maxIncrease.baseSelfSize)} KB → ${formatSize(maxIncrease.prSelfSize)} KB (**${formatDiff(maxIncrease.selfSizeDiff)} KB**)\n\n`; +} + +if (maxDecrease && maxDecrease.selfSizeDiff < 0) { + const moduleName = maxDecrease.modules.length === 0 ? 'Base (no modules)' : maxDecrease.modules.join(', '); + report += `📉 **Largest Decrease:** ${moduleName}\n`; + report += ` - Self Size: ${formatSize(maxDecrease.baseSelfSize)} KB → ${formatSize(maxDecrease.prSelfSize)} KB (**${formatDiff(maxDecrease.selfSizeDiff)} KB**)\n\n`; +} + +// Significant changes table +if (significantChanges.length > 0) { + report += `### Significant Changes (≥ ${THRESHOLD_KB} KB)\n\n`; + report += '| Module(s) | Base (KB) | PR (KB) | Diff (KB) | Gzip Diff (KB) |\n'; + report += '|-----------|-----------|---------|-----------|----------------|\n'; + + for (const diff of significantChanges) { + const moduleName = diff.modules.length === 0 ? 'Base (no modules)' : diff.modules.join(', '); + const emoji = getChangeEmoji(diff.selfSizeDiff); + const status = diff.isNew ? ' 🆕' : diff.isRemoved ? ' 🗑️' : ''; + + report += `| ${emoji} ${moduleName}${status} | ${formatSize(diff.baseSelfSize)} | ${formatSize(diff.prSelfSize)} | **${formatDiff(diff.selfSizeDiff)}** | ${formatDiff(diff.gzipSizeDiff)} |\n`; + } + report += '\n'; +} else { + report += '### Significant Changes\n\n'; + report += `✅ No modules changed by more than ${THRESHOLD_KB} KB.\n\n`; +} + +// New/Removed modules +const newModules = diffs.filter((d) => d.isNew); +const removedModules = diffs.filter((d) => d.isRemoved); + +if (newModules.length > 0) { + report += '### New Modules\n\n'; + for (const m of newModules) { + const moduleName = m.modules.join(', '); + report += `- 🆕 **${moduleName}**: ${formatSize(m.prSelfSize)} KB\n`; + } + report += '\n'; +} + +if (removedModules.length > 0) { + report += '### Removed Modules\n\n'; + for (const m of removedModules) { + const moduleName = m.modules.join(', '); + report += `- 🗑️ **${moduleName}**: was ${formatSize(m.baseSelfSize)} KB\n`; + } + report += '\n'; +} + +// Stats +report += '
\n📊 Full Statistics\n\n'; +report += `- **Modules compared:** ${diffs.length}\n`; +report += `- **Modules with increases:** ${diffs.filter((d) => d.selfSizeDiff > 0).length}\n`; +report += `- **Modules with decreases:** ${diffs.filter((d) => d.selfSizeDiff < 0).length}\n`; +report += `- **Modules unchanged:** ${diffs.filter((d) => d.selfSizeDiff === 0).length}\n`; +report += '
\n'; + +// Write report +fs.writeFileSync(outputFile, report); +console.log(`Comparison report written to ${outputFile}`); + +// Output summary for CI +console.log('\n--- Summary ---'); +console.log(`Significant changes (>= ${THRESHOLD_KB} KB): ${significantChanges.length}`); +let hasChanges = false; +if (maxIncrease && maxIncrease.selfSizeDiff > 0) { + const moduleName = maxIncrease.modules.length === 0 ? 'Base (no modules)' : maxIncrease.modules.join(', '); + console.log(`Largest increase: ${moduleName} (+${formatSize(maxIncrease.selfSizeDiff)} KB)`); + hasChanges = true; +} +if (maxDecrease && maxDecrease.selfSizeDiff < 0) { + const moduleName = maxDecrease.modules.length === 0 ? 'Base (no modules)' : maxDecrease.modules.join(', '); + console.log(`Largest decrease: ${moduleName} (${formatSize(maxDecrease.selfSizeDiff)} KB)`); + hasChanges = true; +} +if (!hasChanges) { + console.log('No module size changes detected.'); +} diff --git a/scripts/ci/merge-module-size-results.mjs b/scripts/ci/merge-module-size-results.mjs new file mode 100644 index 00000000000..ec235ef0d1a --- /dev/null +++ b/scripts/ci/merge-module-size-results.mjs @@ -0,0 +1,69 @@ +#!/usr/bin/env node +import fs from 'node:fs'; +import path from 'node:path'; + +/** + * Merge multiple module-size-results shard files into a single file. + * Each shard file includes the base module (empty modules array) at the start, + * so we need to deduplicate by module key. + * + * Usage: node merge-module-size-results.mjs + */ + +const shardsDir = process.argv[2]; +const outputFile = process.argv[3]; + +if (!shardsDir || !outputFile) { + console.error('Usage: node merge-module-size-results.mjs '); + process.exit(1); +} + +function getModuleKey(modules) { + return modules.length === 0 ? '__base__' : modules.sort().join('+'); +} + +// Find all JSON files in the shards directory +const files = fs + .readdirSync(shardsDir) + .filter((f) => f.endsWith('.json')) + .sort(); // Sort to ensure consistent ordering + +if (files.length === 0) { + console.error(`No JSON files found in ${shardsDir}`); + process.exit(1); +} + +console.log(`Found ${files.length} shard files to merge`); + +// Merge results, deduplicating by module key +const mergedMap = new Map(); + +for (const file of files) { + const filePath = path.join(shardsDir, file); + const content = fs.readFileSync(filePath, 'utf8'); + const results = JSON.parse(content); + + console.log(` - ${file}: ${results.length} entries`); + + for (const result of results) { + const key = getModuleKey(result.modules); + // Keep the first occurrence (all shards should have consistent base values) + if (!mergedMap.has(key)) { + mergedMap.set(key, result); + } + } +} + +// Convert map to array and sort for consistent output +const merged = Array.from(mergedMap.values()); + +// Sort: base module first, then by module name +merged.sort((a, b) => { + if (a.modules.length === 0) return -1; + if (b.modules.length === 0) return 1; + return getModuleKey(a.modules).localeCompare(getModuleKey(b.modules)); +}); + +// Write merged results +fs.writeFileSync(outputFile, JSON.stringify(merged, null, 2)); +console.log(`Merged ${merged.length} unique entries to ${outputFile}`); diff --git a/testing/module-size/moduleValidator.ts b/testing/module-size/moduleValidator.ts index 50472841636..5aaf228a9e3 100644 --- a/testing/module-size/moduleValidator.ts +++ b/testing/module-size/moduleValidator.ts @@ -73,7 +73,8 @@ function validateSizes() { } // testSuites.print(); - process.exit(testSuites.hasFailures() ? 1 : 0); // Return a non-zero exit code + // Disable exit code so that CI does not fail on size warnings as these should now be reviewed on the PR + // process.exit(testSuites.hasFailures() ? 1 : 0); // Return a non-zero exit code } validateSizes(); From 5ec8644ba2b30058fda94584b4c4ccd8340be0b0 Mon Sep 17 00:00:00 2001 From: Stephen Cooper Date: Mon, 12 Jan 2026 10:36:45 +0000 Subject: [PATCH 3/7] AG-16432 fix new bundle task (#12858) * AG-16432 Validate task has run ok * Correct checkout for action --- .github/workflows/module-size-comparison.yml | 53 +++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/.github/workflows/module-size-comparison.yml b/.github/workflows/module-size-comparison.yml index cfc0b2fe230..d505c537b90 100644 --- a/.github/workflows/module-size-comparison.yml +++ b/.github/workflows/module-size-comparison.yml @@ -119,8 +119,8 @@ jobs: - name: Checkout for actions uses: actions/checkout@v4 with: - sparse-checkout: .github/actions ref: ${{ needs.prepare.outputs.head-sha }} + fetch-depth: 1 - name: Run module size tests uses: ./.github/actions/module-size-test @@ -145,8 +145,8 @@ jobs: - name: Checkout for actions uses: actions/checkout@v4 with: - sparse-checkout: .github/actions ref: ${{ needs.prepare.outputs.head-sha }} + fetch-depth: 1 - name: Run module size tests uses: ./.github/actions/module-size-test @@ -320,3 +320,52 @@ jobs: name: module-size-comparison-${{ env.PR_NUMBER }} path: ./comparison-report.md retention-days: 30 + + # Status check job - this is what branch protection should require + # It ensures the workflow properly blocks PRs when jobs fail + status: + name: Module Size Status + runs-on: ubuntu-latest + needs: [prepare, check-base-cache, module-size-pr, module-size-base, compare-and-report] + if: always() && needs.prepare.outputs.is-draft != 'true' + steps: + - name: Check job results + run: | + echo "Checking job results..." + echo "prepare: ${{ needs.prepare.result }}" + echo "check-base-cache: ${{ needs.check-base-cache.result }}" + echo "module-size-pr: ${{ needs.module-size-pr.result }}" + echo "module-size-base: ${{ needs.module-size-base.result }}" + echo "compare-and-report: ${{ needs.compare-and-report.result }}" + + # prepare must succeed + if [[ "${{ needs.prepare.result }}" != "success" ]]; then + echo "::error::prepare job failed" + exit 1 + fi + + # check-base-cache must succeed + if [[ "${{ needs.check-base-cache.result }}" != "success" ]]; then + echo "::error::check-base-cache job failed" + exit 1 + fi + + # module-size-pr must succeed + if [[ "${{ needs.module-size-pr.result }}" != "success" ]]; then + echo "::error::module-size-pr job failed" + exit 1 + fi + + # module-size-base must succeed or be skipped (skipped when cache hit) + if [[ "${{ needs.module-size-base.result }}" != "success" && "${{ needs.module-size-base.result }}" != "skipped" ]]; then + echo "::error::module-size-base job failed" + exit 1 + fi + + # compare-and-report must succeed + if [[ "${{ needs.compare-and-report.result }}" != "success" ]]; then + echo "::error::compare-and-report job failed" + exit 1 + fi + + echo "All required jobs completed successfully!" From 6ab4727ea3ad95ab012944ff1797fcbc0f9e509a Mon Sep 17 00:00:00 2001 From: Tak Tran Date: Mon, 12 Jan 2026 12:04:48 +0000 Subject: [PATCH 4/7] AG-3390 - Resolve snyk issues with rulesync and angular (#12859) * AG-3390 - Add snyk commands to all project * AG-3390 - Resolve snyk issues with rulesync and angular --- .snyk | 7 +++++++ documentation/ag-grid-docs/.snyk | 5 +++++ .../github/actions/snyk-scan-workspaces/action.yml | 1 + packages/ag-grid-angular/.snyk | 13 +++++++++++++ testing/accessibility/.snyk | 13 +++++++++++++ testing/module-size-angular/.snyk | 13 +++++++++++++ utilities/all/project.json | 14 +++++++++++++- 7 files changed, 65 insertions(+), 1 deletion(-) diff --git a/.snyk b/.snyk index 24f47b105a9..a78deb25fa1 100644 --- a/.snyk +++ b/.snyk @@ -72,4 +72,11 @@ ignore: developer machines expires: 2026-06-08T00:00:00.000Z created: 2025-01-05T10:10:00.000Z + SNYK-JS-MODELCONTEXTPROTOCOLSDK-14871802: + - 'rulesync@5.2.0 > @modelcontextprotocol/sdk@1.25.1': + reason: >- + rulesync is only used during development so an attack would only affect + developer machines + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-05T10:10:00.000Z patch: {} diff --git a/documentation/ag-grid-docs/.snyk b/documentation/ag-grid-docs/.snyk index 18fdd63242f..322334c2881 100644 --- a/documentation/ag-grid-docs/.snyk +++ b/documentation/ag-grid-docs/.snyk @@ -41,4 +41,9 @@ ignore: qs is not used on the server expires: 2026-06-08T00:00:00.000Z created: 2025-01-05T10:10:00.000Z + SNYK-JS-PREACT-14897824: + - 'react-instantsearch@7.22.1 > instantsearch.js@4.86.1 > preact@10.27.2': + reason: We don't use preact + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-12T10:00:00.000Z patch: {} diff --git a/external/ag-shared/github/actions/snyk-scan-workspaces/action.yml b/external/ag-shared/github/actions/snyk-scan-workspaces/action.yml index 988376cccae..4ecf9077145 100644 --- a/external/ag-shared/github/actions/snyk-scan-workspaces/action.yml +++ b/external/ag-shared/github/actions/snyk-scan-workspaces/action.yml @@ -43,6 +43,7 @@ runs: env: SNYK_TOKEN: ${{ inputs.snyk_token }} with: + # NOTE: Keep this in sync with `all/project.json` args: --yarn-workspaces --dev --strict-out-of-sync=false --severity-threshold=${{ inputs.severity_threshold }} --exclude=${{ inputs.exclude }} --remote-repo-url=${{ inputs.remote_repo_url }} --json > ${{ inputs.output_dir }}${{ inputs.snyk_file }} - name: Convert snyk report to CTRF report diff --git a/packages/ag-grid-angular/.snyk b/packages/ag-grid-angular/.snyk index 218eab4d72b..3efc24d4101 100644 --- a/packages/ag-grid-angular/.snyk +++ b/packages/ag-grid-angular/.snyk @@ -74,4 +74,17 @@ ignore: developer machines expires: 2026-06-08T00:00:00.000Z created: 2025-01-05T10:10:00.000Z + SNYK-JS-ANGULARCOMPILER-14908872: + - '@angular/compiler@18.2.14': + reason: >- + @angular/compiler is only used during development so an attack would only affect + developer machines + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-12T10:10:00.000Z + SNYK-JS-ANGULARCORE-14908871: + - '@angular/core@18.2.14': + reason: >- + @angular/core is not used for SVG elements + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-12T10:10:00.000Z patch: {} diff --git a/testing/accessibility/.snyk b/testing/accessibility/.snyk index 96bcc74adad..01774f555e2 100644 --- a/testing/accessibility/.snyk +++ b/testing/accessibility/.snyk @@ -12,4 +12,17 @@ ignore: reason: Only used for testing expires: 2026-06-08T00:00:00.000Z created: 2025-12-03T12:11:00.007Z + SNYK-JS-ANGULARCOMPILER-14908872: + - '@angular/compiler@19.2.15': + reason: >- + @angular/compiler is only used during development so an attack would only affect + developer machines + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-12T10:10:00.000Z + SNYK-JS-ANGULARCORE-14908871: + - '@angular/core@19.2.15': + reason: >- + @angular/core is not used for SVG elements + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-12T10:10:00.000Z patch: {} diff --git a/testing/module-size-angular/.snyk b/testing/module-size-angular/.snyk index 6dc15dd3774..6a0c4ca170e 100644 --- a/testing/module-size-angular/.snyk +++ b/testing/module-size-angular/.snyk @@ -55,4 +55,17 @@ ignore: developer machines expires: 2026-06-08T00:00:00.000Z created: 2025-01-05T10:10:00.000Z + SNYK-JS-ANGULARCOMPILER-14908872: + - '@angular/compiler@19.2.15': + reason: >- + @angular/compiler is only used during development so an attack would only affect + developer machines + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-12T10:10:00.000Z + SNYK-JS-ANGULARCORE-14908871: + - '@angular/core@19.2.15': + reason: >- + @angular/core is not used for SVG elements + expires: 2026-06-08T00:00:00.000Z + created: 2025-01-12T10:10:00.000Z patch: {} diff --git a/utilities/all/project.json b/utilities/all/project.json index 2a59ce9a5df..77459aff181 100644 --- a/utilities/all/project.json +++ b/utilities/all/project.json @@ -224,7 +224,19 @@ "unlink": { "command": "./scripts/removeLocalDeps.sh" }, - "snykIgnore": { + "snyk:test": { + "command": "npx snyk test --yarn-workspaces --dev --strict-out-of-sync=false --severity-threshold=high --exclude=.nx,project.json,dist --remote-repo-url=https://github.com/ag-grid/ag-grid", + "options": { + "cwd": "{workspaceRoot}" + } + }, + "snyk:code": { + "command": "npx snyk code test", + "options": { + "cwd": "{workspaceRoot}" + } + }, + "snyk:ignore": { "command": "node ./external/ag-shared/scripts/snyk/getSnykIgnore.mjs", "options": { "cwd": "{workspaceRoot}" From a5c9d67fe383f7157b36eddff668abaa733860e6 Mon Sep 17 00:00:00 2001 From: Alan Treadway Date: Mon, 12 Jan 2026 12:12:23 +0000 Subject: [PATCH 5/7] Fix Gemini MCP config. (#12860) --- .rulesync/mcp.json | 1 - 1 file changed, 1 deletion(-) diff --git a/.rulesync/mcp.json b/.rulesync/mcp.json index c8de2ca8ed8..c9c44db88a8 100644 --- a/.rulesync/mcp.json +++ b/.rulesync/mcp.json @@ -1,7 +1,6 @@ { "mcpServers": { "sequential-thinking": { - "type": "stdio", "command": "yarn", "args": ["run", "--silent", "mcp-server-sequential-thinking"], "env": {} From cfc1df5f0baa3a7405a236f16504a087129901b6 Mon Sep 17 00:00:00 2001 From: Guilherme Lopes Date: Mon, 12 Jan 2026 09:13:30 -0300 Subject: [PATCH 6/7] AG-16229 - [formulas-cell-editor] - fixed integration with Row Numbers (#12854) * AG-16229 - [formula-cell-editor] - row numbers integration * code refactor * detect static formulas correctly * fix whitespace --- .../src/rangeSelection/rangeService.ts | 35 ++- .../src/widgets/agFormulaInputField.ts | 194 +++++++++----- .../widgets/formulaInputRangeSyncFeature.ts | 248 ++++++++++++------ 3 files changed, 333 insertions(+), 144 deletions(-) diff --git a/packages/ag-grid-enterprise/src/rangeSelection/rangeService.ts b/packages/ag-grid-enterprise/src/rangeSelection/rangeService.ts index 0f7f7f15592..f3ed6de4aee 100644 --- a/packages/ag-grid-enterprise/src/rangeSelection/rangeService.ts +++ b/packages/ag-grid-enterprise/src/rangeSelection/rangeService.ts @@ -414,13 +414,17 @@ export class RangeService extends BeanStub implements NamedBean, IRangeService { return; } - const containingRange = this.findContainingRange({ - columns, - startRow: cell, - endRow: cell, - }); + const containingRange = isRowNumber + ? this.findContainingRange({ + columns, + startRow: cell, + endRow: cell, + }) + : undefined; + const isMultiRangeRemoval = + isRowNumber && !!containingRange && isMultiRange && (event.ctrlKey || event.metaKey); - if (isRowNumber && isMultiRange && containingRange) { + if (isMultiRangeRemoval && containingRange) { this.removeRowFromRowNumberRange(cell, containingRange); } else { this.setRangeToCell(cell, isMultiRange); @@ -744,7 +748,13 @@ export class RangeService extends BeanStub implements NamedBean, IRangeService { return; } + // Normalize the selection mode so explicit column lists are respected. + this.setSelectionMode(false); + this.removeAllCellRanges(true); + const rowNumbersEnabled = _isRowNumbers(this.beans); + const allDataColumns = rowNumbersEnabled ? this.getColumnsFromModel(this.visibleCols.allCols) ?? [] : []; + let hasAllColumnsRange = false; for (const cellRange of cellRanges) { if (cellRange.columns && cellRange.startRow) { @@ -764,9 +774,22 @@ export class RangeService extends BeanStub implements NamedBean, IRangeService { }); } + if ( + rowNumbersEnabled && + !hasAllColumnsRange && + allDataColumns.length > 0 && + cellRange.columns && + cellRange.columns.length === allDataColumns.length + ) { + hasAllColumnsRange = allDataColumns.every((column) => cellRange.columns!.includes(column)); + } + this.cellRanges.push(cellRange); } + // Restore row-number selection mode if any range spans all data columns. + this.setSelectionMode(rowNumbersEnabled && hasAllColumnsRange); + this.dispatchChangedEvent(false, true); } diff --git a/packages/ag-grid-enterprise/src/widgets/agFormulaInputField.ts b/packages/ag-grid-enterprise/src/widgets/agFormulaInputField.ts index 8d3f455c9ad..f2d0472eee2 100644 --- a/packages/ag-grid-enterprise/src/widgets/agFormulaInputField.ts +++ b/packages/ag-grid-enterprise/src/widgets/agFormulaInputField.ts @@ -71,24 +71,14 @@ export class AgFormulaInputField extends AgContentEditableField< if (!isFormula) { // Plain values: render as simple text with no token parsing or range syncing. - this.formulaColorByRef.clear(); - this.renderPlainValue(text); - const res = this.setEditorValue(text, silent); - this.dispatchLocalEvent({ type: 'fieldValueChanged' as any }); + this.applyPlainValue(text, { silent, dispatch: true }); this.rangeSyncFeature?.onValueUpdated(text, hasFormulaPrefix); - return res; + return this; } - this.updateFormulaColorsFromValue(text); - this.renderFormula({ - value: text, - currentValue: this.getCurrentValue(), - }); - // We render tokens ourselves, so avoid the base class' setValue (which would re-render) - // and delegate that task to setEditorValue to keep our cached value and the superclass in sync. - const res = this.setEditorValue(text, silent); + this.applyFormulaValue(text, { currentValue: this.getCurrentValue(), silent }); this.rangeSyncFeature?.onValueUpdated(text, hasFormulaPrefix); - return res; + return this; } public getCurrentValue(): string { @@ -236,22 +226,12 @@ export class AgFormulaInputField extends AgContentEditableField< const { isFormula, hasFormulaPrefix } = this.getFormulaState(serialized); if (!isFormula) { - this.formulaColorByRef.clear(); - this.renderPlainValue(serialized, caret); - this.setEditorValue(serialized); - this.dispatchLocalEvent({ type: 'fieldValueChanged' as any }); + this.applyPlainValue(serialized, { caret, dispatch: true }); this.rangeSyncFeature?.onValueUpdated(serialized, hasFormulaPrefix); return; } - this.updateFormulaColorsFromValue(serialized); - this.renderFormula({ - currentValue, - value: serialized, - caret: caret ?? undefined, - }); - this.setEditorValue(serialized); - this.dispatchLocalEvent({ type: 'fieldValueChanged' as any }); + this.applyFormulaValue(serialized, { currentValue, caret: caret ?? undefined, dispatch: true }); this.rangeSyncFeature?.onValueUpdated(serialized, hasFormulaPrefix); } @@ -267,20 +247,57 @@ export class AgFormulaInputField extends AgContentEditableField< const value = this.getCurrentValue(); const updatedValue = value.slice(0, valueOffset) + ref + value.slice(valueOffset + replaceLen); const tokenIndex = getTokenMatchAtOffset(updatedValue, valueOffset)?.index ?? null; - const previousRef = this.updateLastTokenTracking(ref, caretOffset, valueOffset); - - this.updateFormulaColorsFromValue(updatedValue); - this.setEditorValue(updatedValue); - this.renderFormula({ + let previousRef: string | undefined; + this.applyFormulaValueChange({ currentValue: value, - value: updatedValue, + nextValue: updatedValue, caret: caretOffset + ref.length, + updateTracking: () => { + previousRef = this.updateLastTokenTracking(ref, caretOffset, valueOffset); + }, }); - this.dispatchLocalEvent({ type: 'fieldValueChanged' as any }); return { previousRef, tokenIndex }; } + public removeTokenRef(ref: string, tokenIndex?: number | null): boolean { + const value = this.getCurrentValue(); + const matches = getRefTokenMatches(value); + let token: TokenMatch | undefined; + + if (tokenIndex != null) { + token = matches.find((match) => match.index === tokenIndex); + if (token && token.ref !== ref) { + token = undefined; + } + } + + if (!token) { + token = matches.find((match) => match.ref === ref); + } + + if (!token) { + return false; + } + + const updated = value.slice(0, token.start) + value.slice(token.end); + const caretBase = this.selectionCaretOffset ?? token.start; + const caret = Math.min(caretBase, updated.length); + this.applyFormulaValueChange({ + currentValue: value, + nextValue: updated, + caret, + updateTracking: () => { + this.lastTokenValueOffset = null; + this.lastTokenValueLength = null; + this.lastTokenCaretOffset = caret; + this.lastTokenRef = undefined; + }, + }); + + return true; + } + public applyRangeInsert(ref: string): { action: RangeInsertAction; previousRef?: string; @@ -339,15 +356,14 @@ export class AgFormulaInputField extends AgContentEditableField< const value = this.getCurrentValue(); const updated = value.slice(0, token.start) + nextRef + value.slice(token.end); - this.updateFormulaColorsFromValue(updated); - this.updateLastTokenTracking(nextRef, token.start, token.start); - this.setEditorValue(updated); - this.renderFormula({ + this.applyFormulaValueChange({ currentValue: value, - value: updated, + nextValue: updated, caret: token.start + nextRef.length, + updateTracking: () => { + this.updateLastTokenTracking(nextRef, token.start, token.start); + }, }); - this.dispatchLocalEvent({ type: 'fieldValueChanged' as any }); return { previousRef: token.ref, tokenIndex: token.index }; } @@ -376,31 +392,33 @@ export class AgFormulaInputField extends AgContentEditableField< private getTokenInsertOffsets(isNew: boolean): { caretOffset: number; valueOffset: number } | null { // Use cached offsets while dragging ranges so caret doesn't jump between events. - const contentElement = this.getContentElement(); - const caretOffset = - this.selectionCaretOffset ?? - getCaretOffset(contentElement, this.getCurrentValue()) ?? - this.currentValue.length; - const valueOffset = - isNew || this.lastTokenValueOffset == null - ? this.getValueOffsetFromCaret(caretOffset) - : this.lastTokenValueOffset; - - if (valueOffset == null) { - return null; - } - - return { caretOffset, valueOffset }; + return this.getCaretOffsets(this.getCurrentValue(), { + useCachedCaret: true, + useCachedValueOffset: !isNew, + }); } - private getCaretOffsets(value: string): { caretOffset: number; valueOffset: number } | null { + private getCaretOffsets( + value: string, + options: { useCachedCaret: boolean; useCachedValueOffset: boolean } = { + useCachedCaret: false, + useCachedValueOffset: false, + } + ): { caretOffset: number; valueOffset: number } | null { // Snapshot the caret position in both caret units and raw string offsets. - const caretOffset = getCaretOffset(this.getContentElement(), value); + const contentElement = this.getContentElement(); + const caretOffset = options.useCachedCaret + ? this.selectionCaretOffset ?? getCaretOffset(contentElement, value) ?? this.currentValue.length + : getCaretOffset(contentElement, value); if (caretOffset == null) { return null; } - const valueOffset = this.getValueOffsetFromCaret(caretOffset); + const valueOffset = + options.useCachedValueOffset && this.lastTokenValueOffset != null + ? this.lastTokenValueOffset + : this.getValueOffsetFromCaret(caretOffset); + if (valueOffset == null) { return null; } @@ -425,6 +443,57 @@ export class AgFormulaInputField extends AgContentEditableField< return { isFormula, hasFormulaPrefix }; } + private dispatchValueChanged(): void { + this.dispatchLocalEvent({ type: 'fieldValueChanged' as any }); + } + + private applyPlainValue( + value: string, + params: { caret?: number | null; silent?: boolean; dispatch?: boolean } + ): void { + this.formulaColorByRef.clear(); + this.renderPlainValue(value, params.caret); + this.setEditorValue(value, params.silent); + if (params.dispatch) { + this.dispatchValueChanged(); + } + } + + private applyFormulaValue( + value: string, + params: { currentValue?: string; caret?: number | null; silent?: boolean; dispatch?: boolean } + ): void { + this.updateFormulaColorsFromValue(value); + this.renderFormula({ + value, + currentValue: params.currentValue ?? this.getCurrentValue(), + caret: params.caret ?? undefined, + }); + // We render tokens ourselves, so avoid the base class' setValue (which would re-render) + // and delegate that task to setEditorValue to keep our cached value and the superclass in sync. + this.setEditorValue(value, params.silent); + if (params.dispatch) { + this.dispatchValueChanged(); + } + } + + private applyFormulaValueChange(params: { + currentValue: string; + nextValue: string; + caret: number; + updateTracking?: () => void; + }): void { + this.updateFormulaColorsFromValue(params.nextValue); + params.updateTracking?.(); + this.setEditorValue(params.nextValue); + this.renderFormula({ + currentValue: params.currentValue, + value: params.nextValue, + caret: params.caret, + }); + this.dispatchValueChanged(); + } + public replaceTokenRef( previousRef: string, nextRef: string, @@ -467,16 +536,15 @@ export class AgFormulaInputField extends AgContentEditableField< this.formulaColorByRef.set(nextRef, colorIndex); } const updated = value.slice(0, valueOffset) + nextRef + value.slice(valueOffset + previousRef.length); - this.updateFormulaColorsFromValue(updated); const resolvedIndex = getTokenIndex(token); - this.updateLastTokenTracking(nextRef, caretOffset, valueOffset); - this.setEditorValue(updated); - this.renderFormula({ + this.applyFormulaValueChange({ currentValue: value, - value: updated, + nextValue: updated, caret: caretOffset + nextRef.length, + updateTracking: () => { + this.updateLastTokenTracking(nextRef, caretOffset, valueOffset); + }, }); - this.dispatchLocalEvent({ type: 'fieldValueChanged' }); return resolvedIndex ?? tokenIndex ?? null; } diff --git a/packages/ag-grid-enterprise/src/widgets/formulaInputRangeSyncFeature.ts b/packages/ag-grid-enterprise/src/widgets/formulaInputRangeSyncFeature.ts index b1b51efc8a0..cd1845c2e1b 100644 --- a/packages/ag-grid-enterprise/src/widgets/formulaInputRangeSyncFeature.ts +++ b/packages/ag-grid-enterprise/src/widgets/formulaInputRangeSyncFeature.ts @@ -115,6 +115,59 @@ export class FormulaInputRangeSyncFeature extends BeanStub { this.trackedRanges.clear(); } + private getLiveRanges(): CellRange[] { + return this.beans.rangeSvc?.getCellRanges() ?? []; + } + + private withSuppressedRangeEvents(action: () => void): void { + this.suppressRangeEvents = true; + action(); + this.suppressRangeEvents = false; + } + + private setCellRangesSilently(ranges: CellRange[]): void { + const rangeSvc = this.beans.rangeSvc; + if (!rangeSvc) { + return; + } + this.withSuppressedRangeEvents(() => rangeSvc.setCellRanges(ranges)); + } + + private getColorIndexForTokenOrRef(ref: string, tokenIndex?: number | null): number | null { + return this.field.getColorIndexForToken(tokenIndex ?? null) ?? this.field.getColorIndexForRef(ref); + } + + private normaliseRefForComparison(ref: string | null | undefined): string | null { + if (!ref) { + return null; + } + const trimmed = ref.endsWith(':') ? ref.slice(0, -1) : ref; + return trimmed.replace(/\$/g, '').toUpperCase(); + } + + private tagRangeColor(range: CellRange, ref: string, colorIndex: number | null): boolean { + const { rangeClass } = getColorClassesForRef(ref, colorIndex); + if (range.colorClass === rangeClass) { + return false; + } + tagRangeWithFormulaColor(range, ref, colorIndex); + return true; + } + + private trackRange(range: CellRange, ref: string, tokenIndex?: number | null): void { + const existing = this.trackedRanges.get(range); + const nextTokenIndex = tokenIndex !== undefined ? tokenIndex : existing?.tokenIndex ?? null; + + if (!existing) { + this.addTrackedRef(ref); + } else if (existing.ref !== ref) { + this.removeTrackedRef(existing.ref); + this.addTrackedRef(ref); + } + + this.trackedRanges.set(range, { ref, tokenIndex: nextTokenIndex }); + } + private syncRangesFromFormula(value?: string | null): void { // Keep grid ranges in sync with the current refs in the editor text. // This is the "source of truth" pass: it creates/removes ranges to match tokens, @@ -152,7 +205,7 @@ export class FormulaInputRangeSyncFeature extends BeanStub { return; } - const liveRanges = new Set(rangeSvc.getCellRanges() ?? []); + const liveRanges = new Set(this.getLiveRanges()); for (const [range, tracked] of this.trackedRanges.entries()) { if (!liveRanges.has(range)) { this.trackedRanges.delete(range); @@ -188,16 +241,9 @@ export class FormulaInputRangeSyncFeature extends BeanStub { for (let i = 0; i < rangesForRef.length && i < tokenIndices.length; i++) { const range = rangesForRef[i]; const tokenIndex = tokenIndices[i]; - const tracked = this.trackedRanges.get(range); - if (!tracked || tracked.tokenIndex !== tokenIndex) { - this.trackedRanges.set(range, { ref, tokenIndex }); - } - - const colorIndex = this.field.getColorIndexForToken(tokenIndex) ?? this.field.getColorIndexForRef(ref); - const { rangeClass } = getColorClassesForRef(ref, colorIndex); - - if (range.colorClass !== rangeClass) { - tagRangeWithFormulaColor(range, ref, colorIndex); + this.trackRange(range, ref, tokenIndex); + const colorIndex = this.getColorIndexForTokenOrRef(ref, tokenIndex); + if (this.tagRangeColor(range, ref, colorIndex)) { reTagged = true; } } @@ -227,6 +273,17 @@ export class FormulaInputRangeSyncFeature extends BeanStub { return; } + if (event.started) { + // Remember caret so we can restore it after any selection-driven edits. + this.field.rememberCaret(); + } + + if (this.handleRemovedRangeTokens()) { + this.field.restoreCaretAfterToken(); + this.refocusEditingCell(); + return; + } + // If an existing range was resized, update its token instead of inserting a new one. if (this.updateTrackedRangeTokens()) { return; @@ -239,11 +296,6 @@ export class FormulaInputRangeSyncFeature extends BeanStub { return; } - if (event.started) { - // Remember caret so we can restore it after inserting/replacing tokens. - this.field.rememberCaret(); - } - if (event.started && event.finished) { const { action, previousRef, tokenIndex } = this.field.applyRangeInsert(ref); @@ -334,26 +386,17 @@ export class FormulaInputRangeSyncFeature extends BeanStub { if (!params) { return undefined; } - this.suppressRangeEvents = true; - created = rangeSvc.addCellRange(params); - this.suppressRangeEvents = false; + this.withSuppressedRangeEvents(() => { + created = rangeSvc.addCellRange(params); + }); } else { created = this.findLatestRangeForRef(ref, true) ?? this.findLatestRangeForRef(ref, false); } if (created) { - const colorIndex = this.field.getColorIndexForToken(tokenIndex) ?? this.field.getColorIndexForRef(ref); - tagRangeWithFormulaColor(created, ref, colorIndex); - - const existing = this.trackedRanges.get(created); - if (!existing) { - this.addTrackedRef(ref); - } else if (existing.ref !== ref) { - this.removeTrackedRef(existing.ref); - this.addTrackedRef(ref); - } - - this.trackedRanges.set(created, { ref, tokenIndex: tokenIndex ?? existing?.tokenIndex ?? null }); + const colorIndex = this.getColorIndexForTokenOrRef(ref, tokenIndex); + this.tagRangeColor(created, ref, colorIndex); + this.trackRange(created, ref, tokenIndex); this.refreshRangeStyling(); } @@ -361,7 +404,7 @@ export class FormulaInputRangeSyncFeature extends BeanStub { } private findLatestRangeForRef(ref: string, skipTracked: boolean): CellRange | undefined { - const ranges = this.beans.rangeSvc?.getCellRanges() ?? []; + const ranges = this.getLiveRanges(); for (let i = ranges.length - 1; i >= 0; i--) { const range = ranges[i]; if (rangeToRef(this.beans, range) !== ref) { @@ -378,28 +421,22 @@ export class FormulaInputRangeSyncFeature extends BeanStub { private tagLatestRangeForRef(ref: string, tokenIndex?: number | null): void { // The newest range is the one the user just clicked/dragged. - const { beans, field, trackedRanges } = this; - const ranges = beans.rangeSvc?.getCellRanges(); - const latest = ranges?.length ? _last(ranges) : null; + const { trackedRanges } = this; + const ranges = this.getLiveRanges(); + const latest = ranges.length ? _last(ranges) : null; if (!latest) { return; } const trackedIndex = trackedRanges.get(latest)?.tokenIndex ?? null; - const colorIndex = field.getColorIndexForToken(tokenIndex ?? trackedIndex) ?? field.getColorIndexForRef(ref); - - tagRangeWithFormulaColor(latest, ref, colorIndex); + const colorIndex = this.getColorIndexForTokenOrRef(ref, tokenIndex ?? trackedIndex); + this.tagRangeColor(latest, ref, colorIndex); this.refreshRangeStyling(); } private discardLatestRangeForRef(ref: string): void { - const rangeSvc = this.beans.rangeSvc; - if (!rangeSvc) { - return; - } - - const ranges = rangeSvc.getCellRanges() ?? []; + const ranges = this.getLiveRanges(); if (!ranges.length) { return; } @@ -414,9 +451,7 @@ export class FormulaInputRangeSyncFeature extends BeanStub { return; } - this.suppressRangeEvents = true; - rangeSvc.setCellRanges(ranges.slice(0, -1)); - this.suppressRangeEvents = false; + this.setCellRangesSilently(ranges.slice(0, -1)); } private ensureTrackedRangeColors(): boolean { @@ -425,13 +460,10 @@ export class FormulaInputRangeSyncFeature extends BeanStub { // it only re-tags colors for whatever ranges currently exist in the grid. // Some color logic mirrors syncRangesFromFormula on purpose to keep overlays // correct even when external range updates bypass the formula sync. - const rangeSvc = this.beans.rangeSvc; - - if (!rangeSvc) { + const ranges = this.getLiveRanges(); + if (!ranges.length) { return false; } - - const ranges = rangeSvc.getCellRanges(); let reTagged = false; for (const range of ranges) { @@ -453,22 +485,93 @@ export class FormulaInputRangeSyncFeature extends BeanStub { continue; } - const { rangeClass } = getColorClassesForRef(ref, colorIndex); - - if (range.colorClass !== rangeClass) { - tagRangeWithFormulaColor(range, ref, colorIndex); + if (this.tagRangeColor(range, ref, colorIndex)) { reTagged = true; } if (!this.trackedRanges.has(range)) { - this.trackedRanges.set(range, { ref, tokenIndex: tracked?.tokenIndex ?? null }); - this.addTrackedRef(ref); + this.trackRange(range, ref, tracked?.tokenIndex ?? null); } } return reTagged; } + private handleRemovedRangeTokens(): boolean { + // If a tracked range was removed via selection (e.g. Ctrl/Cmd click), drop its token. + if (!this.beans.rangeSvc || this.trackedRanges.size === 0) { + return false; + } + + const value = this.field.getCurrentValue(); + const tokens = getRefTokensFromText(value).filter((token) => token.ref !== this.editingCellRef); + if (!tokens.length) { + return false; + } + + const liveRanges = this.getLiveRanges(); + const liveSet = new Set(liveRanges); + const liveCounts = new Map(); + + for (const range of liveRanges) { + const ref = rangeToRef(this.beans, range); + if (!ref || ref === this.editingCellRef) { + continue; + } + liveCounts.set(ref, (liveCounts.get(ref) ?? 0) + 1); + } + + const pendingRemovals = new Map(); + for (const token of tokens) { + pendingRemovals.set(token.ref, (pendingRemovals.get(token.ref) ?? 0) + 1); + } + for (const [ref, tokenCount] of Array.from(pendingRemovals.entries())) { + const liveCount = liveCounts.get(ref) ?? 0; + const remaining = tokenCount - liveCount; + if (remaining > 0) { + pendingRemovals.set(ref, remaining); + } else { + pendingRemovals.delete(ref); + } + } + + if (!pendingRemovals.size) { + return false; + } + + const removals: Array<{ range: CellRange; tracked: TrackedRange }> = []; + for (const [range, tracked] of Array.from(this.trackedRanges.entries())) { + if (liveSet.has(range)) { + continue; + } + const remaining = pendingRemovals.get(tracked.ref) ?? 0; + if (remaining <= 0) { + continue; + } + pendingRemovals.set(tracked.ref, remaining - 1); + removals.push({ range, tracked }); + } + + if (!removals.length) { + return false; + } + + removals.sort((a, b) => (b.tracked.tokenIndex ?? -1) - (a.tracked.tokenIndex ?? -1)); + + let removed = false; + for (const { range, tracked } of removals) { + removed = this.field.removeTokenRef(tracked.ref, tracked.tokenIndex ?? null) || removed; + this.trackedRanges.delete(range); + this.removeTrackedRef(tracked.ref); + } + + if (removed) { + this.syncRangesFromFormula(this.field.getCurrentValue()); + } + + return removed; + } + private refreshRangeStyling(): void { // Trigger a lightweight refresh so overlays pick up any updated classes. const { eventSvc } = this.beans; @@ -505,15 +608,10 @@ export class FormulaInputRangeSyncFeature extends BeanStub { return; } - const { rangeSvc } = this.beans; - if (rangeSvc) { - const ranges = rangeSvc.getCellRanges() ?? []; - const remaining = ranges.filter((candidate) => candidate !== range); - if (remaining.length !== ranges.length) { - this.suppressRangeEvents = true; - rangeSvc.setCellRanges(remaining); - this.suppressRangeEvents = false; - } + const ranges = this.getLiveRanges(); + const remaining = ranges.filter((candidate) => candidate !== range); + if (remaining.length !== ranges.length) { + this.setCellRangesSilently(remaining); } this.trackedRanges.delete(range); @@ -561,12 +659,12 @@ export class FormulaInputRangeSyncFeature extends BeanStub { private updateTrackedRangeTokens(): boolean { // When a tracked range changes, update the corresponding token text. - const rangeSvc = this.beans.rangeSvc; - if (!rangeSvc) { + if (!this.beans.rangeSvc) { return false; } - const ranges = rangeSvc.getCellRanges(); + const ranges = this.getLiveRanges(); + const editingRef = this.normaliseRefForComparison(this.editingCellRef); let updated = false; for (const range of ranges) { @@ -577,7 +675,9 @@ export class FormulaInputRangeSyncFeature extends BeanStub { const previousRef = tracked.ref; const nextRef = rangeToRef(this.beans, range); - if (!nextRef || nextRef === previousRef || nextRef === this.editingCellRef) { + const normalisedPrevious = this.normaliseRefForComparison(previousRef); + const normalisedNext = this.normaliseRefForComparison(nextRef); + if (!nextRef || !normalisedNext || normalisedNext === normalisedPrevious || normalisedNext === editingRef) { continue; } @@ -595,10 +695,8 @@ export class FormulaInputRangeSyncFeature extends BeanStub { continue; } - tagRangeWithFormulaColor(range, nextRef, colorIndex); - this.trackedRanges.set(range, { ref: nextRef, tokenIndex: replacedIndex ?? tracked.tokenIndex ?? null }); - this.removeTrackedRef(previousRef); - this.addTrackedRef(nextRef); + this.tagRangeColor(range, nextRef, colorIndex); + this.trackRange(range, nextRef, replacedIndex ?? tracked.tokenIndex ?? null); updated = true; } From 93a9caac81a5fdf0171573a34b2765fd53732cad Mon Sep 17 00:00:00 2001 From: Stephen Cooper Date: Mon, 12 Jan 2026 12:14:32 +0000 Subject: [PATCH 7/7] AG-16432 Fix release comp task (#12861) --- .github/workflows/module-size-vs-release.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/module-size-vs-release.yml b/.github/workflows/module-size-vs-release.yml index 1da22203212..a518f053d26 100644 --- a/.github/workflows/module-size-vs-release.yml +++ b/.github/workflows/module-size-vs-release.yml @@ -97,8 +97,9 @@ jobs: - name: Checkout for actions uses: actions/checkout@v4 with: - sparse-checkout: .github/actions ref: latest + fetch-depth: 1 + - name: Run module size tests uses: ./.github/actions/module-size-test @@ -123,8 +124,8 @@ jobs: - name: Checkout for actions uses: actions/checkout@v4 with: - sparse-checkout: .github/actions ref: latest + fetch-depth: 1 - name: Run module size tests uses: ./.github/actions/module-size-test