Skip to content

server.js: Add --config flag for different local webpack config files#4613

Merged
julienw merged 3 commits into
firefox-devtools:mainfrom
kazarmy:server.js-config-flag
May 30, 2023
Merged

server.js: Add --config flag for different local webpack config files#4613
julienw merged 3 commits into
firefox-devtools:mainfrom
kazarmy:server.js-config-flag

Conversation

@kazarmy

@kazarmy kazarmy commented May 11, 2023

Copy link
Copy Markdown
Contributor

This pr adds a --config/-c flag to server.js to allow use of local webpack config files other than webpack.local-config.js.

This addition is primarily to support scripts that invoke server.js to safely override the settings in webpack.local-config.js, since modifying webpack.local-config.js directly is fraught with danger (e.g. what happens if the invoking script or server.js abruptly terminates?). Specifically, this addition is to support the request here (hopefully I've understood correctly) to have a separate Node.js script run server.js.

Another use case is to easily switch between different webpack config files that perhaps open different browsers. But honestly this is secondary to the above.

@codecov

codecov Bot commented May 29, 2023

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (main@8bcde7f). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 5ee627a differs from pull request most recent head 85a009d. Consider uploading reports for the commit 85a009d to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4613   +/-   ##
=======================================
  Coverage        ?   88.61%           
=======================================
  Files           ?      294           
  Lines           ?    26095           
  Branches        ?     7035           
=======================================
  Hits            ?    23125           
  Misses          ?     2765           
  Partials        ?      205           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@julienw julienw self-requested a review May 30, 2023 08:32

@julienw julienw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the patch!

My main concern is about the security of this: indeed require will gladly execute anything that's fed to it. But I think it's fine as long as we don't accept environment variables to provide this information.

Also it's not 100% clear what you'll do with that to support my comment #4452 (review) but I'll wait and see :-)

@julienw julienw enabled auto-merge (squash) May 30, 2023 13:54
@julienw julienw merged commit 04a2eba into firefox-devtools:main May 30, 2023
@canova canova mentioned this pull request Jun 20, 2023
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