You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/04/25 08:40:07 UTC

[PR] fix(docs): use public package path and named imports in component snippets [superset]

rusackas opened a new pull request, #39649:
URL: https://github.com/apache/superset/pull/39649

   ## Summary
   
   Targets #38486 β€” fixes the import snippets the component docs generator emits.
   
   The generator was displaying the internal Storybook alias (`@superset/components`) and inferring default-vs-named import style from the source story file. But the public package re-exports components as **named** exports from `@superset-ui/core/components`:
   
   ```ts
   // packages/superset-ui-core/src/components/index.ts
   export { default as ProgressBar, type ProgressBarProps } from './ProgressBar';
   ```
   
   So a user copy-pasting the doc snippet for `ProgressBar`, `Slider`, `Tabs`, `Tree`, or `TableCollection` would have hit a runtime import error β€” the snippet was both pointing at a non-existent package and using a default import for a named export.
   
   ## Changes
   
   - `docs/scripts/generate-superset-components.mjs`: when the source uses the `@superset/` Storybook alias, render the import using `docImportPrefix` (`@superset-ui/core/components`) and force a named import.
   - Regenerated the 53 affected component MDX files via `node scripts/generate-superset-components.mjs`. Every diff is a single-line change to the `## Import` block.
   
   ## Test plan
   
   - [x] Ran the generator locally; only the `## Import` lines changed.
   - [ ] Confirm Netlify preview renders the component pages.
   
   πŸ€– Generated with [Claude Code](https://claude.com/claude-code)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(docs): use public package path and named imports in component snippets [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #39649:
URL: https://github.com/apache/superset/pull/39649#discussion_r3144344132


##########
docs/scripts/generate-superset-components.mjs:
##########
@@ -1048,6 +1048,18 @@ function generateMDX(component, storyContent) {
   // Use resolved import path if available, otherwise fall back to source config
   const componentImportPath = resolvedImportPath || sourceConfig.importPrefix;
 
+  // The displayed import in user docs should reflect the public package path,
+  // not the internal storybook alias.
+  const docImportPath = sourceConfig.importPrefix.startsWith('@superset/')
+    ? sourceConfig.docImportPrefix
+    : componentImportPath;
+
+  // When the source uses the internal storybook alias, the public package
+  // re-exports components as named exports (e.g. `export { default as Foo }`),
+  // so users must use named imports even when the story uses a default import.
+  const useDefaultImport =
+    isDefaultExport && !sourceConfig.importPrefix.startsWith('@superset/');

Review Comment:
   Addressed in 7278a22 β€” the generator now walks the public barrel (following one level of `export * from './X'`) and only emits the `## Import` snippet when the component is actually re-exported from `@superset-ui/core/components`. `TableCollection` isn't in the barrel, so its import section is omitted rather than emitting an invalid snippet. I considered subpath imports like `@superset-ui/core/components/TableCollection`, but those aren't part of the public package surface either, so omission felt safer than advertising an unsupported path.



##########
docs/scripts/generate-superset-components.mjs:
##########
@@ -185,6 +185,76 @@ const SKIP_STORIES = [
 ];
 
 
+/**
+ * Collect the set of value names exported from a barrel file, following
+ * `export * from './X'` re-exports one level deep. Used to verify that a
+ * component the docs claim is importable is actually re-exported from the
+ * public package entry point.
+ */
+function collectBarrelExports(barrelPath, visited = new Set()) {
+  const exports = new Set();
+  if (!fs.existsSync(barrelPath) || visited.has(barrelPath)) return exports;
+  visited.add(barrelPath);
+
+  const content = fs.readFileSync(barrelPath, 'utf8');
+
+  for (const m of content.matchAll(/export\s+\{([\s\S]*?)\}(?:\s+from\s+['"][^'"]+['"])?/g)) {

Review Comment:
   Ha, fair β€” regex-driven barrel parsing is firmly in Perl territory. I kept it lightweight (no AST parse, no extra deps) since it only runs at docs-generation time and the inputs are our own well-formed barrel files. Happy to swap in a real parser if it starts misbehaving on edge cases.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(docs): use public package path and named imports in component snippets [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39649:
URL: https://github.com/apache/superset/pull/39649#issuecomment-4318566481

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(docs): use public package path and named imports in component snippets [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #39649:
URL: https://github.com/apache/superset/pull/39649#issuecomment-4318568547

   ### <span aria-hidden="true">βœ…</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">πŸ”¨</span> Latest commit | 06130870d4906efc47c8958df30dd774afb1895f |
   |<span aria-hidden="true">πŸ”</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69ec7de8c8a2bb00081741f9 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-39649--superset-docs-preview.netlify.app](https://deploy-preview-39649--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">πŸ“±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTM5NjQ5LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.mSKKkHksXYFu6VvJKN8D89sJvHuOfCNiu85xZdoY-90)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-39649--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(docs): use public package path and named imports in component snippets [superset]

Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber merged PR #39649:
URL: https://github.com/apache/superset/pull/39649


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(docs): use public package path and named imports in component snippets [superset]

Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #39649:
URL: https://github.com/apache/superset/pull/39649#discussion_r3143656302


##########
docs/scripts/generate-superset-components.mjs:
##########
@@ -185,6 +185,76 @@ const SKIP_STORIES = [
 ];
 
 
+/**
+ * Collect the set of value names exported from a barrel file, following
+ * `export * from './X'` re-exports one level deep. Used to verify that a
+ * component the docs claim is importable is actually re-exported from the
+ * public package entry point.
+ */
+function collectBarrelExports(barrelPath, visited = new Set()) {
+  const exports = new Set();
+  if (!fs.existsSync(barrelPath) || visited.has(barrelPath)) return exports;
+  visited.add(barrelPath);
+
+  const content = fs.readFileSync(barrelPath, 'utf8');
+
+  for (const m of content.matchAll(/export\s+\{([\s\S]*?)\}(?:\s+from\s+['"][^'"]+['"])?/g)) {

Review Comment:
   These functions are almost a Perl script in disguise, with all of these text wrangling ;)
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(docs): use public package path and named imports in component snippets [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #39649:
URL: https://github.com/apache/superset/pull/39649#discussion_r3141719132


##########
docs/scripts/generate-superset-components.mjs:
##########
@@ -1048,6 +1048,18 @@ function generateMDX(component, storyContent) {
   // Use resolved import path if available, otherwise fall back to source config
   const componentImportPath = resolvedImportPath || sourceConfig.importPrefix;
 
+  // The displayed import in user docs should reflect the public package path,
+  // not the internal storybook alias.
+  const docImportPath = sourceConfig.importPrefix.startsWith('@superset/')
+    ? sourceConfig.docImportPrefix
+    : componentImportPath;
+
+  // When the source uses the internal storybook alias, the public package
+  // re-exports components as named exports (e.g. `export { default as Foo }`),
+  // so users must use named imports even when the story uses a default import.
+  const useDefaultImport =
+    isDefaultExport && !sourceConfig.importPrefix.startsWith('@superset/');

Review Comment:
   **🟠 Architect Review β€” HIGH**
   
   The generator now assumes that all components using the '@superset/' Storybook alias are named exports from '@superset-ui/core/components', but at least TableCollection is not exported from 'packages/superset-ui-core/src/components/index.ts'. As a result, the generated snippet `import { TableCollection } from '@superset-ui/core/components';` in the TableCollection docs is invalid and will fail in normal copy‑paste usage.
   
   **Suggestion:** Before forcing imports from the barrel '@superset-ui/core/components', validate that the component is actually exported there. If it is not, either generate a subpath import such as `@superset-ui/core/components/TableCollection` (matching the published package structure) or skip/flag that component in docs generation until it is publicly re-exported from the barrel.
   
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20an%20%2A%2AArchitect%20%2F%20Logical%20Review%2A%2A%20comment%20left%20during%20a%20code%20review.%20These%20reviews%20are%20first-class%2C%20important%20findings%20%E2%80%94%20not%20optional%20suggestions.%20Do%20NOT%20dismiss%20this%20as%20a%20%27big%20architectural%20change%27%20just%20because%20the%20title%20says%20architect%20review%3B%20most%20of%20these%20can%20be%20resolved%20with%20a%20small%2C%20localized%20fix%20once%20the%20intent%20is%20understood.%0A%0A%2A%2APath%3A%2A%2A%20docs%2Fscripts%2Fgenerate-superset-components.mjs%0A%2A%2ALine%3A%2A%2A%201053%3A1061%0A%2A%2AComment%3A%2A%2A%0A%09%2AHIGH%3A%20The%20generator%20now%20assumes%20that%20all%20components%20using%20the%20%27%40superset%2F%27%20Storybook%20alias%20are%20named%20exports%20from%20%27%40superset-ui%2Fcore%2Fcomponents%27%2C%20but%20at%20least%20TableCollection%20is%20not%20exported%20from%20%27packages%2Fsuperset-ui-core%2
 Fsrc%2Fcomponents%2Findex.ts%27.%20As%20a%20result%2C%20the%20generated%20snippet%20%60import%20%7B%20TableCollection%20%7D%20from%20%27%40superset-ui%2Fcore%2Fcomponents%27%3B%60%20in%20the%20TableCollection%20docs%20is%20invalid%20and%20will%20fail%20in%20normal%20copy%E2%80%91paste%20usage.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AIf%20a%20suggested%20approach%20is%20provided%20above%2C%20use%20it%20as%20the%20authoritative%20instruction.%20If%20no%20explicit%20code%20suggestion%20is%20given%2C%20you%20MUST%20still%20draft%20and%20apply%20your%20own%20minimal%2C%20localized%20fix%20%E2%80%94%20do%20not%20punt%20back%20with%20%27no%20suggestion%20provided%2C%20review%20manually%27.%20Keep%20the%20change%20as%20small%20as%20possible%3A%20add%20a%20guard%20clause%2C%20gate%20on%20a%20loading%20state%2C%20reorder%20an%
 20await%2C%20wrap%20in%20a%20conditional%2C%20etc.%20Do%20not%20refactor%20surrounding%20code%20or%20expand%20scope%20beyond%20the%20finding.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20an%20%2A%2AArchitect%20%2F%20Logical%20Review%2A%2A%20comment%20left%20during%20a%20code%20review.%20These%20reviews%20are%20first-class%2C%20important%20findings%20%E2%80%94%20not%20optional%20suggestions.%20Do%20NOT%20dismiss%20this%20as%20a%20%27big%20architectural%20change%27%20just%20because%20the%20title%20says%20architect%20review%3B%20most%20of%20these%20can%20be%20resolved%20with%20a%20small%2C%20localized%20fix%20
 once%20the%20intent%20is%20understood.%0A%0A%2A%2APath%3A%2A%2A%20docs%2Fscripts%2Fgenerate-superset-components.mjs%0A%2A%2ALine%3A%2A%2A%201053%3A1061%0A%2A%2AComment%3A%2A%2A%0A%09%2AHIGH%3A%20The%20generator%20now%20assumes%20that%20all%20components%20using%20the%20%27%40superset%2F%27%20Storybook%20alias%20are%20named%20exports%20from%20%27%40superset-ui%2Fcore%2Fcomponents%27%2C%20but%20at%20least%20TableCollection%20is%20not%20exported%20from%20%27packages%2Fsuperset-ui-core%2Fsrc%2Fcomponents%2Findex.ts%27.%20As%20a%20result%2C%20the%20generated%20snippet%20%60import%20%7B%20TableCollection%20%7D%20from%20%27%40superset-ui%2Fcore%2Fcomponents%27%3B%60%20in%20the%20TableCollection%20docs%20is%20invalid%20and%20will%20fail%20in%20normal%20copy%E2%80%91paste%20usage.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AIf%20a
 %20suggested%20approach%20is%20provided%20above%2C%20use%20it%20as%20the%20authoritative%20instruction.%20If%20no%20explicit%20code%20suggestion%20is%20given%2C%20you%20MUST%20still%20draft%20and%20apply%20your%20own%20minimal%2C%20localized%20fix%20%E2%80%94%20do%20not%20punt%20back%20with%20%27no%20suggestion%20provided%2C%20review%20manually%27.%20Keep%20the%20change%20as%20small%20as%20possible%3A%20add%20a%20guard%20clause%2C%20gate%20on%20a%20loading%20state%2C%20reorder%20an%20await%2C%20wrap%20in%20a%20conditional%2C%20etc.%20Do%20not%20refactor%20surrounding%20code%20or%20expand%20scope%20beyond%20the%20finding.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent πŸ€– </b></summary>
   
   ```mdx
   This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings β€” not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
   
   **Path:** docs/scripts/generate-superset-components.mjs
   **Line:** 1053:1061
   **Comment:**
   	*HIGH: The generator now assumes that all components using the '@superset/' Storybook alias are named exports from '@superset-ui/core/components', but at least TableCollection is not exported from 'packages/superset-ui-core/src/components/index.ts'. As a result, the generated snippet `import { TableCollection } from '@superset-ui/core/components';` in the TableCollection docs is invalid and will fail in normal copy‑paste usage.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix β€” do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(docs): use public package path and named imports in component snippets [superset]

Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #39649:
URL: https://github.com/apache/superset/pull/39649#discussion_r3143656302


##########
docs/scripts/generate-superset-components.mjs:
##########
@@ -185,6 +185,76 @@ const SKIP_STORIES = [
 ];
 
 
+/**
+ * Collect the set of value names exported from a barrel file, following
+ * `export * from './X'` re-exports one level deep. Used to verify that a
+ * component the docs claim is importable is actually re-exported from the
+ * public package entry point.
+ */
+function collectBarrelExports(barrelPath, visited = new Set()) {
+  const exports = new Set();
+  if (!fs.existsSync(barrelPath) || visited.has(barrelPath)) return exports;
+  visited.add(barrelPath);
+
+  const content = fs.readFileSync(barrelPath, 'utf8');
+
+  for (const m of content.matchAll(/export\s+\{([\s\S]*?)\}(?:\s+from\s+['"][^'"]+['"])?/g)) {

Review Comment:
   This function is almost a Perl script in disguise, with all of these text wrangling ;)
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org