Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix]: Make Plexus demo work again #2538

Merged
merged 4 commits into from
Jan 4, 2025

Conversation

hari45678
Copy link
Contributor

Which problem is this PR solving?

Description of the changes

  • This PR changes the webpack-factory.js that is responsible for generating dev and prod webpack config on the go.
  • There's a .ejs template (ejs is templating engine for html, similar to pug) already. This webpack config uses that template to generate the html file, similar to what react setups have a index.html in their public folder.

How was this change tested?

  • By running npm run start. The following is the demo that plexus showed on visiting localhost:5000
plexus-demo.webm

Checklist

Signed-off-by: Hariom Gupta <guptahariom03082003@gmail.com>
@hari45678 hari45678 requested a review from a team as a code owner January 4, 2025 09:47
@hari45678 hari45678 requested review from albertteoh and removed request for a team January 4, 2025 09:47
@yurishkuro
Copy link
Member

Nice! A question - when plexus is built as a library for the main Jaeger UI, does the build not use webpack-factory.js in the process? If it does, why did you need all these changes to the file?

Copy link

codecov bot commented Jan 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.63%. Comparing base (82f827e) to head (7791a29).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2538   +/-   ##
=======================================
  Coverage   96.63%   96.63%           
=======================================
  Files         255      255           
  Lines        7728     7728           
  Branches     2015     1950   -65     
=======================================
  Hits         7468     7468           
  Misses        260      260           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hari45678
Copy link
Contributor Author

Nice! A question - when plexus is built as a library for the main Jaeger UI, does the build not use webpack-factory.js in the process? If it does, why did you need all these changes to the file?

It does use webpack-factory.js but there are two functions in that file, one for development build, and other for prod/release build.

const FACTORIES = {
development: makeDevConfig,
'layout-worker': makeWorkerConfig,
umd: makeUmdConfig,
};
function makeWebpackConfig(mode) {
const factory = FACTORIES[mode];
if (!factory) {
throw new Error(`Invalid config type: ${mode}`);
}
const { config, rules } = factory();
const baseConfig = makeBaseConfig();
baseConfig.module.rules.push(...rules);
return {
...baseConfig,
...config,
};
}

The changes I have made are to the makeDevConfig function that is only responsible for environments with development flag/mode

"start": "NODE_ENV='development' npm-run-all -ln --serial _tasks/clean/worker _tasks/bundle-worker --parallel '_tasks/bundle-worker --watch' _tasks/dev-server",

@yurishkuro
Copy link
Member

what about if I start the main UI in dev mode, would it not also propagate down to plexus webpack?

Also, what is UMD? Can you add comments to the config for that?

@yurishkuro
Copy link
Member

btw the linter failed

Checking packages/jaeger-ui
    No depcheck issue
Checking packages/plexus
    Unused devDependencies
    ⛔ file-loader
    ⛔ url-loader

Signed-off-by: Hariom Gupta <guptahariom03082003@gmail.com>
@hari45678
Copy link
Contributor Author

what about if I start the main UI in dev mode, would it not also propagate down to plexus webpack?

Starting the main UI in dev mode is completely isolated to plexus webpack. Webpack comes into picture when we run it. As simple as that. Running the main UI in dev mode runs vite and for vite, that whole plexus folder is like just another directory where src files and components are stored

Also, what is UMD? Can you add comments to the config for that?

Some people like/have to use import statements. Some like/have to use require.

To allow require people use require, js libs have to create UMD builds.

Signed-off-by: Hariom Gupta <guptahariom03082003@gmail.com>
@yurishkuro yurishkuro added the changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements label Jan 4, 2025
@yurishkuro yurishkuro changed the title fix: make demo work in plexus [fix]: Make Plexus demo work again Jan 4, 2025
@yurishkuro yurishkuro merged commit 88883c3 into jaegertracing:main Jan 4, 2025
8 checks passed
@yurishkuro
Copy link
Member

Thanks!

@hari45678 hari45678 deleted the plex-demo branch January 5, 2025 06:50
@yurishkuro
Copy link
Member

@hari45678 I just tried running this on main and it didn't work. I went back to this commit, deleted all node_modules, ran npm ci, then npm run start from plexus dir, and I am still getting the same error

[_tasks/dev-server           ] /Users/ysh/dev/jaegertracing/jaeger-ui/node_modules/webpack-dev-server/lib/Server.js:2602
[_tasks/dev-server           ]         throw error;
[_tasks/dev-server           ]         ^
[_tasks/dev-server           ]
[_tasks/dev-server           ] Error: listen EADDRINUSE: address already in use :::5000
[_tasks/dev-server           ]     at Server.setupListenHandle [as _listen2] (node:net:1907:16)
[_tasks/dev-server           ]     at listenInCluster (node:net:1964:12)
[_tasks/dev-server           ]     at Server.listen (node:net:2066:7)
[_tasks/dev-server           ]     at READ_WRITE (/Users/ysh/dev/jaegertracing/jaeger-ui/node_modules/webpack-dev-server/lib/Server.js:3349:23)
[_tasks/dev-server           ]     at new Promise (<anonymous>)
[_tasks/dev-server           ]     at Server.start (/Users/ysh/dev/jaegertracing/jaeger-ui/node_modules/webpack-dev-server/lib/Server.js:3347:7)
[_tasks/dev-server           ]     at async Command.<anonymous> (/Users/ysh/dev/jaegertracing/jaeger-ui/node_modules/@webpack-cli/serve/lib/index.js:158:21)
[_tasks/dev-server           ]     at async Command.parseAsync (/Users/ysh/dev/jaegertracing/jaeger-ui/node_modules/webpack-cli/node_modules/commander/lib/command.js:935:5)
[_tasks/dev-server           ]     at async Command.<anonymous> (/Users/ysh/dev/jaegertracing/jaeger-ui/node_modules/webpack-cli/lib/webpack-cli.js:1356:13)
[_tasks/dev-server           ]     at async Command.parseAsync (/Users/ysh/dev/jaegertracing/jaeger-ui/node_modules/webpack-cli/node_modules/commander/lib/command.js:935:5) {
[_tasks/dev-server           ]   code: 'EADDRINUSE',
[_tasks/dev-server           ]   errno: -48,
[_tasks/dev-server           ]   syscall: 'listen',
[_tasks/dev-server           ]   address: '::',
[_tasks/dev-server           ]   port: 5000
[_tasks/dev-server           ] }
[_tasks/dev-server           ]
[_tasks/dev-server           ] Node.js v22.11.0
[_tasks/dev-server           ] npm error Lifecycle script `_tasks/dev-server` failed with error:
[_tasks/dev-server           ] npm error code 1
[_tasks/dev-server           ] npm error path /Users/ysh/dev/jaegertracing/jaeger-ui/packages/plexus
[_tasks/dev-server           ] npm error workspace @jaegertracing/plexus@0.2.0
[_tasks/dev-server           ] npm error location /Users/ysh/dev/jaegertracing/jaeger-ui/packages/plexus
[_tasks/dev-server           ] npm error command failed
[_tasks/dev-server           ] npm error command sh -c webpack-dev-server --mode $NODE_ENV --config webpack.dev.config.js
ERROR: "_tasks/dev-server" exited with 1.
npm error Lifecycle script `start` failed with error:
npm error code 1
npm error path /Users/ysh/dev/jaegertracing/jaeger-ui/packages/plexus
npm error workspace @jaegertracing/plexus@0.2.0
npm error location /Users/ysh/dev/jaegertracing/jaeger-ui/packages/plexus
npm error command failed
npm error command sh -c NODE_ENV='development' npm-run-all -ln --serial _tasks/clean/worker _tasks/bundle-worker --parallel '_tasks/bundle-worker --watch' _tasks/dev-server

How were you able to run the demo?

@hari45678
Copy link
Contributor Author

@yurishkuro as per this error message, port 5000 is used by some other service on your environment. Can you please try again after closing that service?

@hari45678
Copy link
Contributor Author

I am able to run on main branch as well

@yurishkuro
Copy link
Member

+1, my bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:bugfix-or-minor-feature 🐞 Bug fixes, Minor Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Figure out what yarn start in plexus is supposed to do
2 participants