Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ jobs:
permissions:
id-token: write
contents: read
packages: read
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this, we were not allowed to install the @deepnote/blocks package.

steps:
- name: Checkout
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
Expand Down
46 changes: 44 additions & 2 deletions build/esbuild/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import { lessLoader } from 'esbuild-plugin-less';
import fs from 'fs-extra';
import { getZeroMQPreBuildsFoldersToKeep, getBundleConfiguration, bundleConfiguration } from '../webpack/common';
import ImportGlobPlugin from 'esbuild-plugin-import-glob';
import postcss from 'postcss';
import tailwindcss from '@tailwindcss/postcss';
import autoprefixer from 'autoprefixer';
const plugin = require('node-stdlib-browser/helpers/esbuild/plugin');
const stdLibBrowser = require('node-stdlib-browser');

Expand Down Expand Up @@ -86,7 +89,11 @@ const loader: { [ext: string]: Loader } = {

// https://github.com/evanw/esbuild/issues/20#issuecomment-802269745
// https://github.com/hyrious/esbuild-plugin-style
function style({ minify = true, charset = 'utf8' }: StylePluginOptions = {}): Plugin {
function style({
minify = true,
charset = 'utf8',
enableTailwind = false
}: StylePluginOptions & { enableTailwind?: boolean } = {}): Plugin {
return {
name: 'style',
setup({ onResolve, onLoad }) {
Expand Down Expand Up @@ -132,6 +139,32 @@ function style({ minify = true, charset = 'utf8' }: StylePluginOptions = {}): Pl
}));

onLoad({ filter: /.*/, namespace: 'style-content' }, async (args) => {
// Process with PostCSS/Tailwind if enabled and file exists
if (enableTailwind && args.path.includes('tailwind.css') && fs.existsSync(args.path)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded filename check is fragile.

The check args.path.includes('tailwind.css') assumes a specific filename. Renaming or adding multiple Tailwind files will break processing.

Consider a more flexible approach (e.g., check for @tailwind directives in content or use a pattern/config).

try {
const cssContent = await fs.readFile(args.path, 'utf8');
const result = await postcss([tailwindcss, autoprefixer]).process(cssContent, {
from: args.path,
to: args.path
});

const options = { ...opt, stdin: { contents: result.css, loader: 'css' } };
options.loader = options.loader || {};
// Add the same loaders we add for other places
Object.keys(loader).forEach((key) => {
if (options.loader && !options.loader[key]) {
options.loader[key] = loader[key];
}
});
const { errors, warnings, outputFiles } = await esbuild.build(options);
return { errors, warnings, contents: outputFiles![0].text, loader: 'text' };
} catch (error) {
console.error(`PostCSS processing failed for ${args.path}:`, error);
throw error;
}
}

// Default behavior for other CSS files
const options = { entryPoints: [args.path], ...opt };
options.loader = options.loader || {};
// Add the same loaders we add for other places
Expand All @@ -140,7 +173,9 @@ function style({ minify = true, charset = 'utf8' }: StylePluginOptions = {}): Pl
options.loader[key] = loader[key];
}
});

const { errors, warnings, outputFiles } = await esbuild.build(options);

return { errors, warnings, contents: outputFiles![0].text, loader: 'text' };
});
}
Expand All @@ -158,7 +193,9 @@ function createConfig(
const plugins: Plugin[] = [];
let define: SameShape<BuildOptions, BuildOptions>['define'] = undefined;
if (target === 'web') {
plugins.push(style());
// Enable Tailwind processing for dataframe renderer
const enableTailwind = source.includes(path.join('dataframe-renderer', 'index.ts'));
plugins.push(style({ enableTailwind }));
Comment on lines +196 to +198
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider more robust path matching.

The check source.includes(path.join('dataframe-renderer', 'index.ts')) is fragile if the file is moved or renamed. Consider a more explicit path comparison or document the coupling.

Example:

-        const enableTailwind = source.includes(path.join('dataframe-renderer', 'index.ts'));
+        const enableTailwind = source === path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'dataframe-renderer', 'index.ts');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Enable Tailwind processing for dataframe renderer
const enableTailwind = source.includes(path.join('dataframe-renderer', 'index.ts'));
plugins.push(style({ enableTailwind }));
// Enable Tailwind processing for dataframe renderer
const enableTailwind = source === path.join(
extensionFolder,
'src',
'webviews',
'webview-side',
'dataframe-renderer',
'index.ts'
);
plugins.push(style({ enableTailwind }));
🤖 Prompt for AI Agents
In build/esbuild/build.ts around lines 196 to 198, the current
source.includes(path.join('dataframe-renderer', 'index.ts')) check is brittle;
replace it with a more robust path comparison such as normalizing the path and
testing for a suffix (e.g.,
path.normalize(source).endsWith(path.join('dataframe-renderer','index.ts'))) or
resolving and comparing relative paths from the project root, or match by
basename + parent directory (e.g., path.basename(source) === 'index.ts' &&
path.dirname(path.normalize(source)).endsWith('dataframe-renderer'));
alternatively, if coupling is intentional, add a comment documenting that this
exact path is required and why.

plugins.push(lessLoader());

define = {
Expand Down Expand Up @@ -287,6 +324,11 @@ async function buildAll() {
),
{ target: 'web', watch: watchAll }
),
build(
path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'dataframe-renderer', 'index.ts'),
path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'dataframeRenderer', 'dataframeRenderer.js'),
{ target: 'web', watch: isWatchMode }
),
build(
path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'variable-view', 'index.tsx'),
path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'viewers', 'variableView.js'),
Expand Down
Loading