Feature/datavis nitro submodule by garrity-miepub · Pull Request #149 · mieweb/ui

garrity-miepub · 2026-03-26T22:28:19Z

feat: convert datavis to git submodule, remove wcdatavis

Architecture

DataVis NITRO is now sourced from a git submodule at packages/datavis/ instead of a git URL npm dependency. The datavis code is pre-bundled into the @mieweb/ui/datavis addon entry point (dist/datavis.js) so consumers do not need to install datavis separately.

Package structure

Main package (@mieweb/ui): No datavis code — unaffected consumers pay zero cost for data visualization.
Opt-in addon (@mieweb/ui/datavis): DataVisNITRO components are pre-bundled in dist/datavis.js. Consumers simply import: import { DataVisNitroGrid } from '@mieweb/ui/datavis';
Source: packages/datavis/ (git submodule pinned to commit 2b32e15). Developers must clone with --recurse-submodules or run git submodule update --init.

What changed

Added packages/datavis as a git submodule (.gitmodules)
package.json: datavis dependency changed from git URL to file:./packages/datavis; removed wcdatavis from optionalDependencies and devDependencies
tsup.config.ts: removed datavis, wcdatavis, and jquery from the external list so datavis is bundled from source into the addon
src/components/DataVisNITRO/DataVisNITRO.tsx: updated imports from wcdatavis/src/ to datavis/wcdatavis-lib/src/
src/types/wcdatavis.d.ts: updated module declarations to match new import paths; removed Grid and index.js modules
src/datavis.ts: removed WCDataVis re-export
Deleted src/components/WCDataVis/ entirely

Why wcdatavis was removed

The datavis package includes a vendored wcdatavis-lib/ that exports ComputedView, Source, and other symbols needed by DataVisNITRO. The standalone wcdatavis npm dependency was only needed by the legacy WCDataVis component which has been removed.

Dependencies

jquery remains as an optional dependency because datavis/wcdatavis-lib uses it internally.
wcdatavis is fully removed from all dependency sections.
The packages/datavis/ submodule folder does NOT ship in the npm package (the files field only includes dist/, README.md, LICENSE).

Bump DataVis & DataVis NITRO versions, fix type issues.

## Architecture DataVis NITRO is now sourced from a git submodule at packages/datavis/ instead of a git URL npm dependency. The datavis code is pre-bundled into the @mieweb/ui/datavis addon entry point (dist/datavis.js) so consumers do not need to install datavis separately. ### Package structure - Main package (@mieweb/ui): No datavis code -- unaffected consumers pay zero cost for data visualization. - Opt-in addon (@mieweb/ui/datavis): DataVisNITRO components are pre-bundled in dist/datavis.js. Consumers simply import: import { DataVisNitroGrid } from '@mieweb/ui/datavis'; - Source: packages/datavis/ (git submodule pinned to commit 2b32e15). Developers must clone with --recurse-submodules or run: git submodule update --init ### What changed - Added packages/datavis as a git submodule (.gitmodules) - package.json: datavis dependency changed from git URL to file:./packages/datavis; removed wcdatavis from optionalDependencies and devDependencies - tsup.config.ts: removed datavis, wcdatavis, and jquery from the external list so datavis is bundled from source into the addon - src/components/DataVisNITRO/DataVisNITRO.tsx: updated imports from wcdatavis/src/ to datavis/wcdatavis-lib/src/ (datavis bundles its own vendored copy of the wcdatavis core library) - src/types/wcdatavis.d.ts: updated module declarations to match new import paths; removed Grid and index.js modules (WCDataVis-only) - src/datavis.ts: removed WCDataVis re-export - Deleted src/components/WCDataVis/ entirely ### Why wcdatavis was removed The datavis package includes a vendored wcdatavis-lib/ that exports ComputedView, Source, and other symbols needed by DataVisNITRO. The standalone wcdatavis npm dependency (and its jQuery UI plugin setup) was only needed by the legacy WCDataVis component which has been removed. ### Dependencies - jQuery remains as an optional dependency because datavis/wcdatavis-lib uses it internally. - wcdatavis is fully removed from all dependency sections. - The packages/datavis/ submodule folder does NOT ship in the npm package (the files field only includes dist/, README.md, LICENSE).

Copilot

Pull request overview

Adds a new opt-in @mieweb/ui/datavis entry point that bundles DataVis NITRO from a packages/datavis git submodule, while removing legacy WCDataVis usage and updating build/Storybook wiring accordingly.

Changes:

Add @mieweb/ui/datavis build entry + package export map and new DataVisNITRO wrapper component/stories.
Update type shims for datavis/wcdatavis-lib and jquery, plus Storybook config/assets for local development.
Adjust dependencies/lockfiles for the new datavis sourcing approach (submodule + local file: reference).

Reviewed changes

Copilot reviewed 13 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`tsup.config.ts`	Adds `datavis` entry point and adjusts externals for the build.
`src/types/wcdatavis.d.ts`	Declares modules for `datavis/wcdatavis-lib` imports.
`src/types/jquery.d.ts`	Adds a minimal ambient module declaration for `jquery` + window globals.
`src/index.ts`	Notes DataVis is also exported via a separate entry point.
`src/datavis.ts`	New addon entry re-exporting DataVisNITRO components.
`src/components/DataVisNITRO/index.ts`	Barrel exports for DataVisNITRO components and types.
`src/components/DataVisNITRO/DataVisNITRO.tsx`	New React wrapper around DataVis NITRO grid/view.
`src/components/DataVisNITRO/DataVisNITRO.stories.tsx`	Storybook stories for the new DataVisNITRO components.
`src/components/Address/AddressForm.tsx`	Refactors Google Maps typings to local interfaces.
`pnpm-lock.yaml`	Lockfile updates for new/optional deps (datavis, jquery, font-awesome).
`package.json`	Adds `./datavis` export, adds font-awesome peer, and adds datavis/jquery deps.
`package-lock.json`	Lockfile changes that currently conflict with the PR’s stated dependency changes.
`.storybook/public/sample-data.json`	Adds sample dataset for DataVisNITRO stories.
`.storybook/preview-head.html`	Attempts to load Font Awesome CSS for DataVis icons.
`.storybook/main.ts`	Adds Vite alias/optimizeDeps logic for DataVis-related deps.
`.gitmodules`	Adds `packages/datavis` git submodule definition.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/components/DataVisNITRO/DataVisNITRO.stories.tsx

package.json

.storybook/main.ts

.storybook/preview-head.html

- Update DataVisNITRO story description to reference datavis/wcdatavis-lib instead of the removed wcdatavis package - Move datavis from optionalDependencies to devDependencies only, since the file:./packages/datavis path would fail for npm consumers (the submodule folder is not shipped in the published package) - Guard readDependencyNames in .storybook/main.ts with existsSync and try-catch so Storybook does not crash if a package is not installed; remove the wcdatavis lookup entirely - Switch font-awesome in .storybook/preview-head.html from a /node_modules/ path (unreliable in static builds) to the cdnjs CDN - Add submodules: true to all actions/checkout@v4 steps in ci.yml, release.yml, and codeql.yml so the packages/datavis submodule is initialized during CI runs

cloudflare-workers-and-pages · 2026-03-26T22:58:42Z

Deploying ui with Cloudflare Pages

Latest commit:	`b08f0e0`
Status:	✅ Deploy successful!
Preview URL:	https://d64136ca.ui-6d0.pages.dev
Branch Preview URL:	https://feature-datavis-nitro-submod.ui-6d0.pages.dev

View logs

Copilot

Pull request overview

Copilot reviewed 16 out of 20 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/components/DataVisNITRO/DataVisNITRO.tsx

…scing for totalRows - normalizeTypeInfo: add Array.isArray branch to convert [{field, type, ...}] to keyed map - totalRows: use ?? instead of || so 0 is not treated as falsy Addresses Copilot review feedback on PR #149.

src/types/jquery.d.ts

Copilot

Pull request overview

Copilot reviewed 16 out of 21 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tsup.config.ts

src/components/Address/AddressForm.tsx

package.json

src/components/Address/AddressForm.tsx

Copilot · 2026-04-01T17:46:20Z

src/components/DataVisNITRO/DataVisNITRO.tsx

+import {
+  DataGrid,
+  type DataGridProps,
+  type GridTableDef,
+} from 'datavis/src/components/DataGrid';


The exported DataVisNitroGridProps is built on types imported from deep datavis/src/... paths (DataGridProps, GridTableDef, etc.). Since datavis is only a local devDependency (file:./packages/datavis) and isn’t shipped to consumers, the generated dist/datavis.d.ts can end up referencing modules consumers can’t resolve. To keep the addon self-contained, avoid leaking datavis/src types in exported types (re-declare needed shapes locally) or ensure the DTS build inlines/rolls up those external types into dist/datavis.d.ts.

Copilot

Pull request overview

Copilot reviewed 16 out of 21 changed files in this pull request and generated no new comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 16 out of 19 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

Comments suppressed due to low confidence (1)

src/components/Address/AddressForm.tsx:311

The effect sets up a persistent place_changed listener that calls onChange({ ...value, ...addressData }), but the dependency array only includes googlePlaces?.enabled. This can capture stale value/onChange/googlePlaces callbacks and merge autocomplete data into an outdated value snapshot. Consider either including the referenced values in the deps (and re-initializing the listener when they change) or storing value/onChange in refs that the listener reads from.

    // eslint-disable-next-line react-hooks/exhaustive-deps -- Only re-init when enabled flag changes; other deps are stable refs
  }, [googlePlaces?.enabled]);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T18:59:56Z

src/components/DataVisNITRO/DataVisNITRO.tsx

+function normalizeTypeInfo(typeInfo: unknown): DataVisTypeInfoMap {
+  if (Array.isArray(typeInfo)) {
+    return Object.fromEntries(
+      typeInfo.filter(isObject).map((entry) => [entry.field, entry])
+    ) as DataVisTypeInfoMap;
+  }
+
+  if (!isObject(typeInfo)) {
+    return {};
+  }
+
+  const maybeOrdMap = typeInfo as DataVisTypeInfoOrdMap;
+  if (
+    typeof maybeOrdMap.keys === 'function' &&
+    typeof maybeOrdMap.get === 'function'
+  ) {
+    return Object.fromEntries(
+      maybeOrdMap.keys().map((field) => [field, maybeOrdMap.get?.(field) ?? {}])
+    );
+  }
+
+  return Object.fromEntries(
+    Object.entries(typeInfo).filter(([, value]) => isObject(value))
+  ) as DataVisTypeInfoMap;
+}


This PR introduces substantial new logic in DataVisNITRO.tsx (typeInfo normalization, column resolution/merging, tableDef merging, etc.) but there are no unit tests covering these behaviors. Since the repo runs Vitest with coverage thresholds in CI, consider adding a DataVisNITRO.test.tsx (or focused unit tests for the helper functions) to prevent regressions, especially around normalizeTypeInfo and derived column ordering.

Copilot · 2026-04-01T18:59:56Z

pnpm-lock.yaml

+      datavis-ace:
+        specifier: '=4.0.0-PRE.1'
+        version: 4.0.0-PRE.1


pnpm-lock.yaml lists datavis-ace under the root importer’s dependencies, but package.json only declares it as an (optional) peerDependency and does not include it in dependencies/devDependencies. With pnpm install --frozen-lockfile in CI, this package.json/lockfile mismatch will cause installs to fail. Either add datavis-ace to devDependencies (common when also listed as a peer) or regenerate the lockfile so it matches the current package.json intent.

Suggested change

datavis-ace:

specifier: '=4.0.0-PRE.1'

version: 4.0.0-PRE.1

tvenable-mie and others added 6 commits March 25, 2026 15:25

Creating a component for WC DataVis using layout tag names.

9569e02

Creating a component for DataVis NITRO.

1e36888

Try switching the repos to public so CI can get them.

b271cee

Bump DataVis & DataVis NITRO versions, fix type issues.

Move DataVis NITRO along.

4719d66

Break DataVis into optional addons.

814a580

garrity-miepub requested review from Copilot, horner and wreiske March 26, 2026 22:28

Copilot started reviewing on behalf of garrity-miepub March 26, 2026 22:28 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

src/components/DataVisNITRO/DataVisNITRO.stories.tsx Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

.storybook/main.ts Outdated Show resolved Hide resolved

.storybook/preview-head.html Outdated Show resolved Hide resolved

garrity-miepub marked this pull request as ready for review March 26, 2026 23:11

Copilot AI review requested due to automatic review settings March 26, 2026 23:11

Copilot started reviewing on behalf of garrity-miepub March 26, 2026 23:11 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

src/components/DataVisNITRO/DataVisNITRO.tsx Show resolved Hide resolved

src/components/DataVisNITRO/DataVisNITRO.tsx Outdated Show resolved Hide resolved

horner requested changes Mar 27, 2026

View reviewed changes

src/types/jquery.d.ts Outdated Show resolved Hide resolved

Copilot AI review requested due to automatic review settings April 1, 2026 17:39

Copilot started reviewing on behalf of tvenable-mie April 1, 2026 17:40 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 1, 2026 18:42

Copilot started reviewing on behalf of tvenable-mie April 1, 2026 18:43 View session

tvenable-mie force-pushed the feature/datavis-nitro-submodule branch from 4e920db to df032db Compare April 1, 2026 18:45

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 1, 2026 18:52

Copilot started reviewing on behalf of tvenable-mie April 1, 2026 18:53 View session

Switch to datavis-ace as peer, remove jQuery & FontAwesome deps.

b08f0e0

tvenable-mie force-pushed the feature/datavis-nitro-submodule branch from 672bbe2 to b08f0e0 Compare April 1, 2026 18:57

Copilot AI reviewed Apr 1, 2026

View reviewed changes

horner self-requested a review April 1, 2026 19:15

horner approved these changes Apr 1, 2026

View reviewed changes

garrity-miepub merged commit 2cba6ce into main Apr 1, 2026
10 checks passed

garrity-miepub deleted the feature/datavis-nitro-submodule branch April 1, 2026 19:16

Conversation

garrity-miepub commented Mar 26, 2026

Architecture

Package structure

What changed

Why wcdatavis was removed

Dependencies

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cloudflare-workers-and-pages bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying ui with Cloudflare Pages

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cloudflare-workers-and-pages bot commented Mar 26, 2026 •

edited

Loading