Skip to content

Conversation

goanpeca
Copy link
Collaborator

No description provided.

@goanpeca goanpeca self-assigned this Aug 27, 2025
@goanpeca goanpeca force-pushed the enh/use-vite-on-react branch from b7e5aae to 7c06680 Compare August 27, 2025 05:26
@goanpeca goanpeca marked this pull request as ready for review August 27, 2025 05:26
@goanpeca goanpeca requested review from Copilot and echarles August 27, 2025 05:26
Copilot

This comment was marked as outdated.

@goanpeca goanpeca force-pushed the enh/use-vite-on-react branch from 7c06680 to 22e5cc1 Compare August 27, 2025 05:31
@goanpeca goanpeca requested a review from Copilot August 27, 2025 05:47
Copilot

This comment was marked as outdated.

@goanpeca goanpeca force-pushed the enh/use-vite-on-react branch from 22e5cc1 to 72fb498 Compare August 27, 2025 05:56
@goanpeca goanpeca requested a review from Copilot August 27, 2025 05:56
Copilot

This comment was marked as outdated.

@goanpeca goanpeca force-pushed the enh/use-vite-on-react branch from 72fb498 to 3c7f39e Compare August 27, 2025 06:01
@goanpeca goanpeca requested a review from Copilot August 27, 2025 06:02
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the React package from webpack to Vite as the build tool, updating configuration files and build scripts accordingly.

Key changes:

  • Replace webpack with Vite for both development and production builds
  • Update TypeScript configuration for Vite compatibility
  • Simplify import paths by removing explicit lib/ directory references

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/react/vite.config.ts New Vite configuration with library build support and development server setup
packages/react/tsconfig.json Updated TypeScript config for Vite compatibility (baseUrl and composite changes)
packages/react/package.json Updated build scripts to use Vite instead of webpack and added Vite dependencies
packages/react/index.html New HTML entry point for Vite development server with dynamic module loading
packages/docusaurus-plugin/src/theme/ReactLiveScope/index.tsx Simplified import paths to use package root instead of lib/ subdirectory
packages/docusaurus-plugin/src/theme/JupyterCell.tsx Simplified import paths to use package root instead of lib/ subdirectory
docs/src/theme/ReactLiveScope/index.tsx Simplified import paths to use package root instead of lib/ subdirectory

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

},
outDir: 'lib',
sourcemap: true,
emptyOutDir: false, // Don't clean TypeScript output
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment suggests TypeScript output should be preserved, but with Vite handling the build, this comment may be misleading. Consider clarifying what specific output needs to be preserved or if this setting is still necessary.

Suggested change
emptyOutDir: false, // Don't clean TypeScript output
emptyOutDir: false, // Prevents cleaning the output directory before build; set to false if you need to preserve files from previous builds (e.g., TypeScript output)

Copilot uses AI. Check for mistakes.

Comment on lines +46 to +50
const FORMAT_EXTENSION_MAP: Record<string, string> = {
es: 'js',
cjs: 'cjs',
};
const ext = FORMAT_EXTENSION_MAP[format] || 'js';
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The FORMAT_EXTENSION_MAP constant is only used once in the fileName function. Consider inlining this mapping or moving it outside the function scope for better reusability.

Suggested change
const FORMAT_EXTENSION_MAP: Record<string, string> = {
es: 'js',
cjs: 'cjs',
};
const ext = FORMAT_EXTENSION_MAP[format] || 'js';
const ext = (format === 'cjs') ? 'cjs' : 'js';

Copilot uses AI. Check for mistakes.

// Default entry point constant for better maintainability
const DEFAULT_ENTRY_POINT = 'notebookthemecolormode';

// Dynamically import the entry point based on environment variable
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VITE_ENTRY environment variable is not documented. Add a comment explaining how to use this variable to switch between different entry points.

Suggested change
// Dynamically import the entry point based on environment variable
// Dynamically import the entry point based on environment variable
/**
* The VITE_ENTRY environment variable allows you to switch between different entry points for the application.
*
* To use a specific entry point, set VITE_ENTRY in your environment before starting Vite, e.g.:
* VITE_ENTRY=notebook npm run dev
*
* Valid values for VITE_ENTRY are:
* - app: loads /src/app/App.tsx
* - cell: loads /src/examples/Cell.tsx
* - console: loads /src/examples/Console.tsx
* - filebrowser: loads /src/examples/FileBrowser.tsx
* - notebook: loads /src/examples/Notebook.tsx
* - notebookthemecolormode: loads /src/examples/NotebookThemeColormode.tsx
* - terminal: loads /src/examples/Terminal.tsx
* - viewer: loads /src/examples/Viewer.tsx
*
* If VITE_ENTRY is not set, the default entry point is 'notebookthemecolormode'.
*/

Copilot uses AI. Check for mistakes.

@echarles
Copy link
Member

@goanpeca Does pyodide (NotebookLite example) still work this vite.js config (for ref, I think that config works for pyodide https://github.com/datalayer-examples/jupyter-react-vite-example/blob/main/vite.config.ts)

@goanpeca
Copy link
Collaborator Author

@goanpeca Does pyodide (NotebookLite example) still work this vite.js config (for ref, I think that config works for pyodide https://github.com/datalayer-examples/jupyter-react-vite-example/blob/main/vite.config.ts)

I will check!

@goanpeca goanpeca marked this pull request as draft August 27, 2025 06:13
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.

2 participants