Skip to content

admin-ui: Fix jenkins build error#2419

Merged
duttarnab merged 4 commits intomainfrom
buildError
Nov 5, 2025
Merged

admin-ui: Fix jenkins build error#2419
duttarnab merged 4 commits intomainfrom
buildError

Conversation

@syntrydy
Copy link
Contributor

@syntrydy syntrydy commented Nov 5, 2025

Description

ERROR in ./plugins/auth-server/components/Scopes/ScopeAddPage.test.tsx 51:29
Module parse failed: Unexpected token (51:29)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| })
| 
> const Wrapper = ({ children }: { children: React.ReactNode }) => (
|   <AppTestWrapper>
|     <Provider store={store}>{children}</Provider>
 @ ./plugins/ sync ^.*$ ./auth-server/components/Scopes/ScopeAddPage.test.tsx ./auth-server/components/Scopes/ScopeAddPage.test
 @ ./plugins/PluginMenuResolver.js 155:39-63 169:41-65
 @ ./app/routes/index.tsx 9:0-78 30:21-34 41:29-46
 @ ./app/components/App/AuthenticatedRouteSelector.tsx 3:0-51 18:35-48
 @ ./app/components/App/AppMain.tsx 5:0-70 19:36-62
 @ ./app/components/App/index.ts 2:0-32 3:15-22
 @ ./app/index.tsx 2:0-35 33:34-37

Summary by CodeRabbit

  • Chores
    • Builds now ignore test files in both development and production, keeping bundles lean and preventing test code from entering module graphs.
  • Bug Fixes / Reliability
    • Plugin metadata loading is more selective and resilient, with stricter inclusion rules, safer fallbacks, improved merging/sorting of menus/routes, and better error handling for more stable UI behavior.

@syntrydy syntrydy self-assigned this Nov 5, 2025
@syntrydy syntrydy requested a review from duttarnab as a code owner November 5, 2025 15:13
@syntrydy syntrydy added bug Something isn't working kind-bug Issue or PR is a bug in existing functionality labels Nov 5, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

📝 Walkthrough

Walkthrough

Dev and prod webpack configs updated to ignore test files from loaders and add ignore-loader rules; PluginMenuResolver now restricts dynamic plugin-metadata imports with webpackInclude, uses optional chaining and spread-push merging, replaces sortMenu with sortParentMenu, and adds error handling for metadata loads. (≤50 words)

Changes

Cohort / File(s) Change Summary
Dev Webpack config
admin-ui/config/webpack.config.client.dev.ts
Excluded test patterns from TS/JS loader rules (*.test.ts, *.test.tsx, *.test.js) and added ignore-loader entries to skip those test files in srcDir and pluginsDir.
Prod Webpack config
admin-ui/config/webpack.config.client.prod.ts
Added ignore-loader rules to skip *.test.ts/*.test.tsx files in config.srcDir and config.pluginsDir; minor formatting tweak in plugin name extraction.
Plugin Menu Resolution
admin-ui/plugins/PluginMenuResolver.js
Restricted dynamic imports with webpackInclude for plugin-metadata; switched concatenation to push(...result) merging; replaced sortMenu with sortParentMenu; used optional chaining (default?.menus, default?.routes) with nullish fallbacks; added .catch handlers that log a warning and return empty arrays; updated synchronous require fallback similarly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant DevServer as Dev Server (webpack)
  participant IgnoreLoader as Ignore Loader
  participant TSLoader as TS/JS Loaders
  participant ModuleGraph as Module Graph

  rect rgb(213,245,227)
    note over DevServer,ModuleGraph: Dev build ignores test files during module discovery
  end

  DevServer->>IgnoreLoader: scan srcDir/pluginsDir for files
  IgnoreLoader-->>DevServer: filter out *.test.js / *.test.ts / *.test.tsx
  DevServer->>TSLoader: process remaining .ts/.tsx/.js (node_modules & tests excluded)
  TSLoader-->>ModuleGraph: emit modules
  ModuleGraph-->>DevServer: bundle for dev
Loading
sequenceDiagram
  autonumber
  participant App as App
  participant Importer as Dynamic Importer
  participant PluginFile as plugin-metadata.js/ts
  participant Merger as Menu/Route Aggregator

  rect rgb(255,240,194)
    note over Importer,PluginFile: Dynamic import restricted by webpackInclude
  end

  App->>Importer: request plugin metadata (async)
  Importer->>PluginFile: import(/* webpackInclude: ... */ plugin-metadata)
  PluginFile-->>Importer: metadata (default?.menus / default?.routes) or fail
  alt success
    Importer->>Merger: push(...menus) / push(...routes) then sortParentMenu
    Merger-->>App: aggregated menus/routes
  else failure
    Importer-->>App: log warning, return empty arrays
    Merger-->>App: continue with other plugins
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Areas needing attention:
    • Verify webpack ignore-loader/include patterns and their order to avoid excluding non-test files.
    • Confirm webpackInclude regex matches intended plugin-metadata filenames and build-time bundling behavior.
    • Check that promise .catch fallbacks (empty arrays) preserve downstream assumptions.
    • Ensure replacing sortMenusortParentMenu maintains expected menu ordering.

Poem

🐇
I hopped through builds with nimble feet,
nudged test files gently off the street.
I caught each import, tidy and neat,
pushed menus home and made them meet.
A rabbit's tweak — the bundle's sweet. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'admin-ui: Fix jenkins build error' directly relates to the changeset, which addresses a Jenkins/Webpack build error by excluding test files from webpack processing.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch buildError

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4dca558 and 3af7c14.

📒 Files selected for processing (1)
  • admin-ui/config/webpack.config.client.prod.ts (2 hunks)
🔇 Additional comments (2)
admin-ui/config/webpack.config.client.prod.ts (2)

75-75: Minor formatting adjustment, no functional impact.

The added indentation is a cosmetic change that doesn't affect the plugin name extraction logic.


210-215: Correct fix for the build error—changes verified.

The new ignore-loader rule for TypeScript/TSX test files is properly implemented at lines 210-215, mirroring the existing pattern for JavaScript test files. This complements the babel-loader exclude pattern (line 189) to prevent Webpack from processing test files during the module resolution phase, resolving the "unexpected token" error from ScopeAddPage.test.tsx. Verification confirms no test files are imported in production code.


Comment @coderabbitai help to get the list of available commands and usage tips.

@mo-auto mo-auto added the comp-admin-ui Component affected by issue or PR label Nov 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f760fe5 and ea723f6.

📒 Files selected for processing (1)
  • admin-ui/config/webpack.config.client.dev.ts (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Analyze (python)
🔇 Additional comments (2)
admin-ui/config/webpack.config.client.dev.ts (2)

180-180: LGTM! Babel exclusion pattern correctly updated.

The exclusion pattern now prevents babel-loader from processing .test.ts and .test.tsx files, which addresses the module parse failure. This works in conjunction with the ignore-loader rules below to provide defense-in-depth.


192-203: The webpack ignore-loader fix is valid, but the root cause diagnosis is incorrect.

The ignore-loader rules in the webpack config (lines 192-203) are a correct and defensive approach to prevent test files from being bundled. However, the review comment's assertion that "ScopeAddPage.test.tsx was being imported through plugins/PluginMenuResolver.js" is contradicted by the code:

  • PluginMenuResolver.js uses dynamic imports to load plugin-metadata files only
  • The auth-server plugin-metadata.js imports ScopeAddPage (production file at ./components/Scopes/ScopeAddPage), not the test file
  • No test files are actually imported anywhere in the production codebase

The webpack rules added in this PR are still beneficial as a defensive guard against accidental test file inclusion, but they're solving a different issue than what the review comment describes. The test file is not being imported through the plugin resolver as claimed.

Likely an incorrect or invalid review comment.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 5, 2025

@duttarnab duttarnab merged commit 9008008 into main Nov 5, 2025
9 checks passed
@duttarnab duttarnab deleted the buildError branch November 5, 2025 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working comp-admin-ui Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants