Skip to content

Conversation

abdullahtellioglu
Copy link
Collaborator

Description

The PR introduces a new Vite plugin that identifies properties of each JSX element during the transformation step, which is used by the Copilot Properties Panel in the browser.

How it works (build-time):

  1. For each .tsx module (excluding anything under /generated/), the plugin:
    • Loads the TypeScript Program using the project's tsconfig.
    • Parses the source file and collects distinct JSX opening elements.
    • For each element, resolves the props type via a TypeScript TypeChecker (Resolver).
    • Serializes the resolved properties into a ResponseData payload.

How it works (runtime, via injected code):
2) Adds a global registry under window.Vaadin.copilot.ReactProperties:

  • properties: Record<string, ResponseData> – in-memory map of tagName → ResponseData.
  • registerer(tagName: string, value: ResponseData) – helper to populate properties.
  1. For non-intrinsic elements (custom React components), the plugin tries to attach
    the payload directly to the component function/object on __debugProperties
    (mimicking a "Fiber-adjacent" location) for quick introspection:

    MyComponent.__debugProperties = { error: false, properties: [...] }

    If that attachment isn’t possible (e.g., MyComponent isn't resolvable as a value),
    it falls back to registerer("MyComponent", payload).

  2. For intrinsic elements (e.g., 'div', 'button'), the plugin always registers to the
    global registry via registerer("div", payload).

https://github.com/vaadin/copilot-internal/pull/6576 is the inline version of both client and plugin implementation.

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

Copy link

github-actions bot commented Sep 1, 2025

Test Results

1 266 files  +1  1 266 suites  +1   1h 15m 14s ⏱️ -11s
8 646 tests +5  8 579 ✅ +5  67 💤 ±0  0 ❌ ±0 
9 103 runs   - 3  9 026 ✅  - 3  77 💤 ±0  0 ❌ ±0 

Results for commit c6a03d4. ± Comparison against base commit 2c49cb2.

♻️ This comment has been updated with latest results.

@abdullahtellioglu abdullahtellioglu marked this pull request as ready for review September 1, 2025 12:41
@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Sep 8, 2025
@Artur-
Copy link
Member

Artur- commented Sep 8, 2025

When running this with the Copilot change, I only see this. Am I holding it wrong?
image

@abdullahtellioglu
Copy link
Collaborator Author

When running this with the Copilot change, I only see this. Am I holding it wrong?

Are you sure intrinsic element types are imported? Here, vite tests do not load them so I mimic them here.

It should not happen in the PR where plugin is inline with a test project https://github.com/vaadin/copilot-internal/pull/6576

@Artur-
Copy link
Member

Artur- commented Sep 9, 2025

Are you sure intrinsic element types are imported?

I have no idea what that means. I ran the test project in Copilot.

@abdullahtellioglu abdullahtellioglu marked this pull request as draft September 9, 2025 17:28
Artur- and others added 5 commits September 12, 2025 11:30
Fixed the Vite plugin to correctly find the JSX namespace in all scenarios:
- No React import (automatic JSX runtime with jsx: "react-jsx")
- Default React import (import React from 'react')
- Explicit JSX import (import type { JSX } from 'react')
- Namespace import (import * as React from 'react')

The fix uses multiple strategies to locate JSX namespace and only includes
the current file in the TypeScript program for better performance and
proper module resolution.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@abdullahtellioglu abdullahtellioglu marked this pull request as ready for review September 15, 2025 07:10
Artur-
Artur- previously approved these changes Sep 16, 2025
@Artur-
Copy link
Member

Artur- commented Sep 16, 2025

Did check the performance impact of this?

@abdullahtellioglu
Copy link
Collaborator Author

Did check the performance impact of this?

I did not see any performance impact so far but it would be better to test it with a page with many components and pages. I will create a snapshot locally and perform testing in the Hilla application to save timestamps of the transform operation

@vaadin-bot vaadin-bot added +1.0.0 and removed +0.0.1 labels Sep 17, 2025
@abdullahtellioglu
Copy link
Collaborator Author

Did check the performance impact of this?

Ts.createProgram consumes most of the time in the transform step (~200-500ms). I did a couple of improvements but it never goes under 100ms, nor have the changes affected the performance a lot.

Tried the solution that Claude suggested, and it is almost the same result. It is not possible to reuse the same ts.program with different files, as it is immutable and given files are cached.

@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Sep 17, 2025
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants