Skip to content

Conversation

@amcaplan
Copy link
Contributor

@amcaplan amcaplan commented Nov 6, 2025

WHY are these changes introduced?

This is the final PR in the 8-PR stack that completes the GraphiQL migration by integrating the new React 18 console with the app server.

Context: All previous PRs built the standalone React console. Now we need to:

  • Update the server to serve the built React app instead of using templates
  • Inject runtime configuration securely into the page
  • Remove the old template-based implementation
  • Ensure all existing functionality continues to work

This completes the migration from template-based GraphiQL to a modern React 18 standalone package.

WHAT is this pull request doing?

This PR updates the Express server to serve the new GraphiQL console and removes the old template implementation.

Key Changes:

1. Server Integration (packages/app/src/cli/services/dev/graphiql/server.ts):

Serve Built React App:

// Find the built React app
const graphiqlAssetsDir = await findPathUp(joinPath('assets', 'graphiql'), {
  type: 'directory',
  cwd: moduleDirectory(import.meta.url),
})

// Serve static assets (JS, CSS, Monaco workers)
app.use('/extensions/graphiql/assets', express.static(...))
app.use('/monacoeditorwork', express.static(...))

Secure Config Injection:

const config = {
  apiVersion,          // Current API version
  apiVersions,         // Available versions
  appName,            // App name from dev config
  appUrl,             // App preview URL
  storeFqdn,          // Store domain
  baseUrl,            // Server base URL
  key,                // Optional security key
  query,              // Optional initial query from URL param
}

// SECURITY: Escape < > & using Unicode escapes to prevent XSS
// This prevents script tag breakout: </script><script>alert('xss')</script>
const safeJson = JSON.stringify(config)
  .replace(/</g, '\\u003c')
  .replace(/>/g, '\\u003e')
  .replace(/&/g, '\\u0026')

const configScript = `<script>window.__GRAPHIQL_CONFIG__ = ${safeJson};</script>`
indexHtml = indexHtml.replace('</head>', `${configScript}\n  </head>`)

Why Unicode Escapes?

Attack Prevention Example:

// Without escaping (VULNERABLE):
const config = {query: "</script><script>alert('xss')</script><script>"}
// Result: <script>window.__GRAPHIQL_CONFIG__ = {..."</script><script>alert('xss')...
// Browser executes the injected script!

// With Unicode escaping (SAFE):
const safeJson = JSON.stringify(config).replace(/</g, '\\u003c').replace(/>/g, '\\u003e')
// Result: <script>window.__GRAPHIQL_CONFIG__ = {..."\u003c/script\u003e\u003cscript\u003ealert('xss')...
// Browser treats it as data, not executable code!

Query Parameter Support:

// Support passing initial query via URL
// GET /graphiql?query=...
const query = decodeQueryString(req.query.query as string)

2. New Server Endpoints:

/graphiql/ping (GET):

  • Returns "pong" to indicate server is alive
  • Used by useServerStatus hook for health checks
  • Polls every 2 seconds by default

/graphiql/status (GET):

  • Returns app installation status and store info
  • Checks if token refresh succeeds (app is installed)
  • Returns: {status: 'OK', storeFqdn, appName, appUrl}
  • Used by useServerStatus hook for app status
  • Polls every 5 seconds by default

3. Cleanup - Remove Old Template Implementation:

  • ❌ Deleted graphiql.tsx template (365 lines)
  • ❌ Deleted old style.css (58 lines)
  • ✅ Total: 423 lines of legacy code removed

4. Build Integration:

// packages/app/project.json
{
  "targets": {
    "build": {
      "dependsOn": ["^build"],  // Build graphiql-console first
    }
  }
}
  • App build now depends on graphiql-console build
  • Ensures built assets are available before app builds

5. .gitignore Update:

packages/app/assets/graphiql
  • Ignore built assets (generated by Vite)
  • Assets are built during CI and local development

Files Modified:

  • packages/app/src/cli/services/dev/graphiql/server.ts - Server integration (57 additions, 34 deletions)
  • packages/app/project.json - Build dependency
  • .gitignore - Ignore built assets

Files Deleted:

  • packages/app/src/cli/services/dev/graphiql/templates/graphiql.tsx - Old template (365 lines)
  • packages/app/assets/graphiql/style.css - Old styles (58 lines)

Dependencies

Builds on ALL 7 previous PRs:

Result: 🎉 Migration Complete! The GraphiQL console is now a standalone React 18 package integrated with the app server.

How to test your changes?

Build and Run:

# Build the graphiql-console package
pnpm --filter @shopify/graphiql-console build

# Verify built assets exist
ls -la packages/app/assets/graphiql

# Run the dev server
cd /path/to/shopify-app
dev server

# Open GraphiQL
# The server will log the URL, typically: http://localhost:3457/graphiql

Verify Functionality:

  1. ✅ GraphiQL loads with React UI
  2. ✅ Status badge shows "Running" (green)
  3. ✅ API version selector shows available versions
  4. ✅ Store and app link pills appear
  5. ✅ Monaco syntax highlighting works
  6. ✅ GraphQL queries execute successfully
  7. ✅ Switching API versions re-fetches schema

Test Server Health Monitoring:

  1. While GraphiQL is open, stop the dev server (Ctrl+C)
  2. ✅ Status badge changes to "Disconnected" (red)
  3. ✅ Error banner appears: "The server has been stopped"
  4. Restart the dev server
  5. ✅ Status badge changes back to "Running" (green)
  6. ✅ Error banner disappears

Test Config Injection:
Open browser DevTools console:

console.log(window.__GRAPHIQL_CONFIG__)
// Should show: {apiVersion, apiVersions, appName, appUrl, storeFqdn, baseUrl, ...}

Test Query Parameter:

# Pass initial query via URL
http://localhost:3457/graphiql?query=%7Bshop%7Bname%7D%7D
# GraphiQL should open with "{shop{name}}" query pre-loaded

Security Verification:
Verify config escaping in page source (View Page Source):

<script>window.__GRAPHIQL_CONFIG__ = {..."query":"\u003cscript\u003ealert('test')\u003c/script\u003e"...};</script>
  • Should see \u003c and \u003e instead of < and >
  • No executable script tags should be present

Measuring impact

  • n/a - this completes the GraphiQL migration to React 18

Post-release steps

None - this is a drop-in replacement for the existing GraphiQL implementation.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Migration Summary

This stack successfully migrates GraphiQL from template-based implementation to a modern React 18 standalone package:

Before:

  • 365 lines of Rails-like template code
  • Direct DOM manipulation with vanilla JavaScript
  • No type safety
  • Difficult to test and maintain
  • Limited features

After:

  • Standalone React 18 package with 1,500+ lines of tested code
  • Monaco editor integration for advanced syntax highlighting
  • Full TypeScript type safety
  • Comprehensive test coverage (89 tests, 1,000+ test lines)
  • Security-first design (XSS prevention at multiple layers)
  • Modern development workflow with Vite
  • Responsive design for different screen sizes
  • Real-time server health monitoring
  • Maintainable, testable, and extensible codebase

Security Improvements:

  1. Server-side config escaping (Unicode escapes)
  2. Client-side config validation and sanitization (PR feat(graphiql): add shared types, constants, and validation #6579)
  3. URL allowlist for localhost and Shopify domains
  4. Defense-in-depth approach to XSS prevention

Developer Experience:

  • Hot reload during development
  • Type-checked at compile time
  • Component isolation for easier testing
  • Reusable hooks for common patterns
  • Modern React patterns (hooks, functional components)

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 79.35% 13745/17323
🟡 Branches 73.27% 6728/9183
🟡 Functions 79.5% 3549/4464
🟡 Lines 79.71% 12976/16280

Test suite run success

3447 tests passing in 1401 suites.

Report generated by 🧪jest coverage report action from 8dc20f9

@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from 0c3ce5a to ada77f5 Compare November 6, 2025 16:53
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch 4 times, most recently from 0328b01 to 8dc20f9 Compare November 6, 2025 17:27
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from ada77f5 to 0de0e5b Compare November 6, 2025 17:27
@amcaplan amcaplan changed the base branch from 11-06-feat_graphiql_complete_graphiql_section_and_app_integration to graphite-base/6585 November 6, 2025 21:22
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch from 8dc20f9 to d2ee648 Compare November 6, 2025 21:22
@amcaplan amcaplan changed the base branch from graphite-base/6585 to 11-06-feat_graphiql_complete_graphiql_section_and_app_integration November 6, 2025 21:22
@amcaplan amcaplan changed the base branch from 11-06-feat_graphiql_complete_graphiql_section_and_app_integration to graphite-base/6585 November 6, 2025 21:29
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch from d2ee648 to a20ec99 Compare November 6, 2025 21:29
@amcaplan amcaplan changed the base branch from graphite-base/6585 to 11-06-feat_graphiql_complete_graphiql_section_and_app_integration November 6, 2025 21:29
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from af92c30 to de0599e Compare November 6, 2025 22:14
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch from a20ec99 to 9b31961 Compare November 6, 2025 22:14
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from de0599e to e5b4e41 Compare November 6, 2025 22:27
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch from 9b31961 to 381c5ff Compare November 6, 2025 22:27
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from e5b4e41 to ffeaf36 Compare November 6, 2025 22:46
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch from 381c5ff to 59d489a Compare November 6, 2025 22:46
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from ffeaf36 to 2fed8fb Compare November 6, 2025 22:55
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch from 59d489a to e9873a7 Compare November 6, 2025 22:55
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from 2fed8fb to 286d74c Compare November 6, 2025 23:00
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch 2 times, most recently from 66fe31d to 94990cf Compare November 6, 2025 23:04
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from 286d74c to 48047fb Compare November 6, 2025 23:04
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch 2 times, most recently from ad526cd to 852c655 Compare November 6, 2025 23:21
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from 16851d0 to 530b9a5 Compare November 6, 2025 23:21
Updates the app server to serve the new GraphiQL console with secure
config injection. Removes old template-based implementation.

Server changes:
- Serve built React app from packages/app/assets/graphiql
- Inject runtime config with Unicode escaping for XSS prevention
- Use \u003c, \u003e, \u0026 to prevent script tag breakout
- Support query parameter passing

Cleanup:
- Remove old template implementation (365 lines)
- Remove old CSS styles (58 lines)

Server integration complete with security built-in
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_complete_graphiql_section_and_app_integration branch from 530b9a5 to 8708c64 Compare November 6, 2025 23:34
@amcaplan amcaplan force-pushed the 11-06-feat_graphiql_integrate_console_with_app_server branch from 852c655 to 3e7dadf Compare November 6, 2025 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant