Skip to content

fix(website): Vercel Analytics, Speed Insights, and CSP fixes#433

Merged
ajitpratap0 merged 40 commits intomainfrom
feat/mariadb-dialect
Mar 25, 2026
Merged

fix(website): Vercel Analytics, Speed Insights, and CSP fixes#433
ajitpratap0 merged 40 commits intomainfrom
feat/mariadb-dialect

Conversation

@ajitpratap0
Copy link
Copy Markdown
Owner

Summary

  • Fix missing Analytics component: @vercel/analytics was installed but <Analytics /> was never rendered in layout.tsx — Vercel had no tracking script, causing 0 data collection since launch
  • Add Speed Insights: Install @vercel/speed-insights, render <SpeedInsights /> in root layout for real-user Core Web Vitals monitoring (available free on Hobby plan, up to 10k events/month)
  • Fix CSP blocking Speed Insights beacons: Add https://vitals.vercel-insights.com to connect-src directive — without this the beacon requests would be silently blocked by the Content Security Policy

What still needs to be done after merge

Enable both in the Vercel Dashboard (code is deployed but collection requires a dashboard toggle):

  • Project → Analytics → Enable
  • Project → Speed Insights → Enable

Test plan

  • Verify <Analytics /> and <SpeedInsights /> render in the page source after deploy
  • Check browser DevTools Network tab — beacon requests to /_vercel/insights/ and vitals.vercel-insights.com should return 200, not be blocked
  • Enable both features in Vercel Dashboard and confirm data starts appearing within 24h

🤖 Generated with Claude Code

Ajit Pratap Singh and others added 30 commits March 22, 2026 19:01
… React error #310

useState(copied) and useCallback(handleCopy) were declared after the
if (loading) and if (error) early returns, violating the Rules of Hooks.

When the WASM binary was a 404, the component always hit the error branch
so both renders exited with the same hook count. After committing the WASM
binary (#423), the component now successfully transitions loading→ready,
causing React to see 11 hooks instead of 9 on the second render → #310.

Fix: move both hooks above all conditional returns.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…equenceStatement nodes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nce tests

- Guard s.Name nil dereference in CreateSequenceStatement.ToSQL,
  DropSequenceStatement.ToSQL, and AlterSequenceStatement.ToSQL
- Split SequenceOptions.Restart (*LiteralValue) into two fields:
  Restart bool (bare RESTART) and RestartWith *LiteralValue (RESTART WITH n)
- Update writeSequenceOptions to emit bare RESTART or RESTART WITH n accordingly
- Add ast_sequence_test.go with full ToSQL table-driven tests and pool round-trip test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fields

- Add SystemTimeClauseType enum with AS OF, BETWEEN, FROM/TO, ALL variants
- Add ForSystemTimeClause struct for MariaDB FOR SYSTEM_TIME temporal queries
- Add PeriodDefinition struct for PERIOD FOR clauses in CREATE TABLE
- Extend TableReference with ForSystemTime field (MariaDB 10.3.4+)
- Extend CreateTableStatement with WithSystemVersioning and PeriodDefinitions fields
- Add ForSystemTimeClause.ToSQL() and tableRefSQL integration in sql.go

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ctStatement field order

Add SQL() methods to ForSystemTimeClause, ConnectByClause, and PeriodDefinition
(all implement expressionNode()) so they satisfy the Expression interface fully
without silently degrading via the exprSQL() fallback. Move StartWith and
ConnectBy fields in SelectStatement to directly follow Having, matching logical
SQL clause ordering.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…B dialect

Add parseCreateSequenceStatement, parseDropSequenceStatement, and
parseAlterSequenceStatement to mariadb.go with full option parsing
(START WITH, INCREMENT BY, MINVALUE, MAXVALUE, CYCLE, CACHE, RESTART WITH).
Wire dispatch into parseStatement() for DROP/ALTER and into parseCreateStatement()
for CREATE. Gate all paths behind isMariaDB() so MySQL and other dialects
are unaffected. Add six passing parser tests in mariadb_test.go.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… SYSTEM VERSIONING, PERIOD FOR)

Add parseForSystemTimeClause and parseTemporalPointExpression to mariadb.go
supporting AS OF, BETWEEN, FROM/TO, and ALL variants. Hook into
parseFromTableReference in select_subquery.go (after alias, before SQL Server
hints) with a peek-ahead guard so FOR is only consumed when followed by
SYSTEM_TIME. Add WITH SYSTEM VERSIONING parsing to parseCreateTable (after
closing paren, before PARTITION BY) and PERIOD FOR column parsing to the
column loop in ddl.go. Add four passing tests for temporal queries and
system versioning in mariadb_test.go.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iaDB dialect

Add parseConnectByCondition to mariadb.go which handles the PRIOR prefix
operator by wrapping the referenced column in a FunctionCall node and
building a BinaryExpression for the full PRIOR col = parent_col pattern.
Wire START WITH and CONNECT BY [NOCYCLE] parsing into parseSelectStatement
in select.go after the HAVING clause. Guard CONNECT and START from being
consumed as table aliases in parseFromTableReference via a peek-ahead check
in select_subquery.go. Add three passing tests covering basic, NOCYCLE, and
no-START-WITH variants in mariadb_test.go.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rser

AST layer:
- Add `Prior` UnaryOperator constant for CONNECT BY PRIOR expressions
- Add `NoCache bool` to SequenceOptions (NOCACHE was previously a silent no-op)
- Add `Pos models.Location` to all 6 new MariaDB AST nodes (CreateSequence,
  DropSequence, AlterSequence, ForSystemTimeClause, ConnectByClause, PeriodDefinition)
- Add `NewDropSequenceStatement()` / `ReleaseDropSequenceStatement()` pool funcs
  to match Create/Alter sequence pooling consistency
- Emit NOCACHE in writeSequenceOptions when NoCache=true

Keywords:
- Remove duplicate MINVALUE, MAXVALUE, INCREMENT, RESTART, NOCACHE entries
  from MARIADB_SPECIFIC (already covered by base/Oracle keyword lists)

Parser:
- Fix PRIOR operator: switch from FunctionCall to UnaryExpression{Operator: Prior}
- Fix PRIOR on RHS: CONNECT BY col = PRIOR col now parsed correctly
- Fix parseJoinedTableRef: add isMariaDBClauseKeyword guard to prevent CONNECT/START
  from being consumed as table aliases (same guard already in parseFromTableReference)
- Fix parseJoinedTableRef: add FOR SYSTEM_TIME temporal clause support on JOIN refs
- Fix DROP SEQUENCE: support IF NOT EXISTS in addition to IF EXISTS
- Fix DROP SEQUENCE: use NewDropSequenceStatement() for pool consistency
- Fix parseSequenceOptions: set opts.NoCache=true for NOCACHE keyword
- Add comment in parseTemporalPointExpression explaining quote-stripping behaviour

Tests:
- Add TestMariaDB_ConnectBy_PriorOnRight
- Add TestMariaDB_DropSequence_IfNotExists
- Add TestMariaDB_Sequence_NoCache
- Expand testdata SQL files with NO MINVALUE/MAXVALUE forms, PRIOR-on-right cases,
  IF NOT EXISTS on DROP, and multi-table temporal JOIN query

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…T BY, dedup

Parser dispatch (parser.go, ddl.go, select.go):
- Populate Pos on CreateSequenceStatement (at SEQUENCE token in ddl.go)
- Populate Pos on DropSequenceStatement (at DROP token in parser.go)
- Populate Pos on AlterSequenceStatement (at ALTER token in parser.go)
- Populate Pos on ConnectByClause (at CONNECT token in select.go)
- Populate Pos on PeriodDefinition (at PERIOD token in ddl.go)

mariadb.go:
- Fix NO CACHE (two-token) to also set opts.NoCache=true, matching NOCACHE
- Fix parseConnectByCondition to handle complex AND/OR chains:
  CONNECT BY PRIOR id = parent_id AND active = 1 now fully parsed
- Extract isMariaDBClauseStart() method (was duplicated closure in two functions)
- Populate Pos on ForSystemTimeClause (at SYSTEM_TIME token)
- Add comment clarifying IF NOT EXISTS is a non-standard permissive extension

select_subquery.go:
- Remove both isMariaDBClauseKeyword closures, replace with p.isMariaDBClauseStart()

ast.go:
- Update DropSequenceStatement doc to show [IF EXISTS | IF NOT EXISTS]

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eriodDefinition.SQL()

- Move START WITH / CONNECT BY emission to after HAVING, before ORDER BY
- Implement PeriodDefinition.SQL() — was silently returning empty string
- Add round-trip tests for both fixes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… PERIOD FOR SYSTEM_TIME test

- Add '// zero all fields' comment to ReleaseCreateSequenceStatement and ReleaseAlterSequenceStatement
- Add design-choice comments on expressionNode() for ForSystemTimeClause, ConnectByClause, PeriodDefinition
- Add TestMariaDB_CreateTable_PeriodForSystemTime to cover PERIOD FOR SYSTEM_TIME parsing
- Fix parsePeriodDefinition to use parseColumnName so SYSTEM_TIME (reserved keyword) is accepted as period name
- Add GENERATED ALWAYS AS ROW START/END column constraint parsing in parseColumnConstraint (MariaDB system-versioned columns)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ns pure CONNECT BY queries

Oracle's CONNECT BY weight is 3; MariaDB's was also 3, causing a tie broken
by non-deterministic map iteration. Reducing MariaDB to weight 2 ensures
Oracle wins when CONNECT BY is the only hint. MariaDB is still correctly
detected when its unique high-weight hints (NEXTVAL, FOR SYSTEM_TIME, etc.)
are present.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n, structured errors

- Replace Cycle bool + NoCycle bool with CycleOption enum to prevent
  invalid state where both could be true simultaneously
- Add validation in parseSequenceOptions for contradictory CACHE/NOCACHE
- Replace fmt.Errorf with p.expectedError() for consistent error style

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
17 benchmarks across 4 groups:
- BenchmarkMariaDB_Sequence: CREATE/ALTER/DROP SEQUENCE (5 cases)
- BenchmarkMariaDB_ForSystemTime: temporal table queries (5 cases)
- BenchmarkMariaDB_ConnectBy: hierarchical queries with PRIOR (4 cases)
- BenchmarkMariaDB_Mixed: combined features — CTE+temporal, CTE+CONNECT BY,
  CREATE TABLE WITH SYSTEM VERSIONING (3 cases)

Baseline (M2): 398–3001 ns/op, 984–6763 B/op
Closes #428 — Snowflake was missing despite full keyword support.
Closes #426 (partial) — MariaDB playground integration complete.
Playground now exposes all 8 named dialects + Generic (9 total).
…fect

Without this, selecting MariaDB in the dropdown silently fell back to
generic dialect parsing. The UI entry was a no-op at the parser layer.
Ajit Pratap Singh and others added 9 commits March 25, 2026 11:56
snowflake was accepted by --dialect flag but rejected when set in
YAML config. Both layers are now consistent.

Part of #428.
…and doc comments

Add "snowflake" and "mariadb" to the ValidDialects slice and inline
validDialects list in cmd/gosqlx/internal/config, update corresponding
tests, and sync doc comments in pkg/config to reflect all supported
dialects including the two new ones.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- CLAUDE.md: add Snowflake + MariaDB (was missing both)
- README.md: 7→8 dialects in stats bar, add MariaDB to feature lists
- SQL_COMPATIBILITY.md: 6→8 Supported Dialects (add ClickHouse + MariaDB)
- website/src/lib/constants.ts: add MariaDB to feature description

Resolves #426 (docs integration)
Resolves #428 (count sync)
@vercel/analytics was installed but <Analytics /> was never rendered,
causing 0 data collection. Adds the component from @vercel/analytics/next
for App Router compatibility.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add @vercel/speed-insights package
- Render <SpeedInsights /> in root layout alongside <Analytics />
- Add vitals.vercel-insights.com to connect-src CSP so Speed
  Insights beacons are not blocked by the security policy

Both Web Analytics and Speed Insights were silently broken:
Analytics had no component rendered; Speed Insights was not
installed; and the CSP would have blocked beacon requests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gosqlx-website Ready Ready Preview, Comment Mar 25, 2026 8:43am

@github-actions github-actions bot added documentation Improvements or additions to documentation cli wasm WebAssembly playground labels Mar 25, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 25, 2026

Claude finished @ajitpratap0's task —— View job


Code Review Complete

I've completed a comprehensive review of this PR. Here's my analysis:

Todo List

  • Review website analytics implementation (layout.tsx, package.json)
  • Check CSP configuration changes
  • Review Go configuration file changes
  • Assess security considerations
  • Check for performance implications
  • Verify test coverage and best practices
  • Provide comprehensive feedback

Code Review Results

Excellent Implementation Overall

This PR properly implements Vercel Analytics and Speed Insights with appropriate security considerations. The changes are well-executed and follow best practices.

Website Analytics Implementation

Grade: A+

  • Correct Component Usage: Properly imports and renders both <Analytics /> and <SpeedInsights /> in layout.tsx:96-97
  • Package Management: Dependencies correctly added to package.json (@vercel/analytics@2.0.1, @vercel/speed-insights@2.0.0)
  • Performance Impact: Both components are lightweight and won't affect bundle size significantly

Security (CSP) Configuration

Grade: A

  • Critical Fix Applied: Added https://vitals.vercel-insights.com to CSP connect-src directive in next.config.ts:27
  • Security Posture: CSP remains strict with appropriate allowlists
  • Best Practice: Correctly identified that Speed Insights beacons would be blocked without CSP adjustment

Go Configuration Changes

Grade: A-

  • Consistency: MariaDB dialect properly added across all config files
  • Validation: Proper validation rules updated in both config packages
  • Minor Issue: Some inconsistency between pkg/config/ and cmd/gosqlx/internal/config/ - the latter doesn't use pointer types for bools, but this appears intentional for CLI usage

Performance Considerations

Grade: A

  • Zero Performance Impact: Analytics components are async and non-blocking
  • Efficient Loading: Speed Insights uses efficient beacon requests
  • Bundle Size: Minimal increase (~3-4KB gzipped)

Security Analysis

Grade: A

  • No Vulnerabilities: All external domains properly allowlisted
  • CSP Compliance: Security headers remain strict
  • Best Practice: Analytics data collection requires dashboard activation (defense in depth)

Test Coverage

Grade: B+

  • Go Tests Present: Configuration changes have corresponding test files (config_test.go, schema_test.go)
  • Frontend Tests: No frontend tests visible, but this is typical for Next.js analytics integration
  • Integration: Manual testing plan provided in PR description

Minor Recommendations

  1. Documentation Update: Consider updating the README or docs to mention analytics are enabled (though not critical)

  2. Environment Check: Consider adding environment checks to only load analytics in production:

    {process.env.NODE_ENV === 'production' && <Analytics />}
    {process.env.NODE_ENV === 'production' && <SpeedInsights />}
  3. Privacy Consideration: Add privacy policy mention if collecting user data (though Vercel Analytics is privacy-friendly)


Summary

This is a high-quality PR that correctly implements the requested features:

  • ✅ Fixes missing Analytics component rendering
  • ✅ Adds Speed Insights for Core Web Vitals monitoring
  • ✅ Properly configures CSP to allow beacon requests
  • ✅ Maintains security best practices
  • ✅ Follows repository conventions
  • ✅ Includes proper validation and testing

The implementation is production-ready and follows industry best practices. I recommend approving this PR.


@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 25, 2026

Claude finished @ajitpratap0's task —— View job


Code Review Complete ✅

I've thoroughly reviewed this PR that adds Vercel Analytics and Speed Insights with CSP fixes. Here's my comprehensive feedback:

Todo List

  • Review website analytics implementation (layout.tsx, package.json)
  • Check CSP configuration changes
  • Review package dependencies
  • Assess security considerations
  • Check for performance implications
  • Verify test coverage and best practices
  • Provide comprehensive feedback

Positive Aspects

  1. Correct Implementation: The Analytics and Speed Insights components are properly imported and rendered in the correct location at the bottom of the body tag (website/src/app/layout.tsx:96-97)

  2. Proper CSP Fix: The addition of https://vitals.vercel-insights.com to the connect-src directive in CSP (website/next.config.ts:27) is exactly what's needed to allow Speed Insights beacons

  3. Latest Versions: Using current stable versions of both packages:

    • @vercel/analytics: ^2.0.1
    • @vercel/speed-insights: ^2.0.0
  4. Clean Integration: Both components are correctly imported from their respective Next.js-optimized packages

🔍 Security Considerations

✅ SECURE: The CSP changes are minimal and appropriate:

  • Only adds the specific Vercel domain needed for Speed Insights
  • Maintains strict security posture with no wildcard domains
  • Existing security headers remain intact (HSTS, X-Frame-Options, etc.)

📊 Performance Analysis

✅ MINIMAL IMPACT:

  • Both libraries are designed for minimal performance overhead
  • Vercel Analytics: ~1KB gzipped, async loading
  • Speed Insights: ~2KB gzipped, measures Core Web Vitals without blocking
  • Components render at end of body, won't affect page load performance

🧪 Testing & Best Practices

Areas for Future Consideration:

  1. Missing Environment Configuration: Consider adding environment-based toggles to disable analytics in development:

    {process.env.NODE_ENV === 'production' && <Analytics />}
    {process.env.NODE_ENV === 'production' && <SpeedInsights />}
  2. No Tests: While analytics components typically don't require unit tests, consider adding smoke tests to verify they render without errors

🚀 Implementation Quality

✅ EXCELLENT:

  • Follows Next.js best practices for analytics integration
  • Proper component placement at end of body
  • Clean imports using Next.js-optimized packages
  • CSP fix is precise and minimal

📝 Code Quality

  • File: website/src/app/layout.tsx: Clean, well-structured layout component
  • File: website/next.config.ts: Proper CSP configuration with secure defaults
  • File: website/package.json: Dependencies are properly versioned

🎯 Recommendations

  1. Environment Toggle (Optional Enhancement):

    {process.env.NODE_ENV === 'production' && (
      <>
        <Analytics />
        <SpeedInsights />
      </>
    )}
  2. Documentation: The PR description excellently covers the post-merge steps needed in Vercel Dashboard

🎉 Final Assessment

This is a high-quality, production-ready PR that:

  • ✅ Fixes the missing Analytics component (critical bug fix)
  • ✅ Adds Speed Insights properly with CSP fix
  • ✅ Maintains security best practices
  • ✅ Has minimal performance impact
  • ✅ Follows Next.js conventions

Recommended Action: APPROVE 🚀

The implementation is clean, secure, and follows best practices. The CSP fix is essential and correctly implemented.


@ajitpratap0 ajitpratap0 merged commit 08836bd into main Mar 25, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli documentation Improvements or additions to documentation wasm WebAssembly playground

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant