Skip to content

Conversation

@felixmosh
Copy link
Contributor

@felixmosh felixmosh commented Feb 1, 2025

I need this method for a rsbuild adapter...
When rsbuild restarts the dev-server, this plugin doesn't closes the running server....

@atassis
Copy link
Owner

atassis commented Feb 2, 2025

  1. There are both code clean and logical changes in single commit, makes it harder to review what is done.
  2. I'ld suggest to make some sense in ordering methods, for example, apply method has moved to top and looks like a new method, while _stopServer is placed on its place. I think, it would be better to keep private and public methods separated, if order is changed at all.

@felixmosh
Copy link
Contributor Author

  1. there is no logical changes at all, I've just extracted a repeated code into a stopServer method.
  2. regarding sorting the methods, i will revert the change :]

@felixmosh
Copy link
Contributor Author

How is it now?

Copy link
Owner

@atassis atassis left a comment

Choose a reason for hiding this comment

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

LGTM now

@atassis atassis merged commit f82b36c into atassis:main Feb 2, 2025
@atassis
Copy link
Owner

atassis commented Feb 2, 2025

I am preparing to some medical interventions, will publish a new version in between preparations (today)

@felixmosh
Copy link
Contributor Author

Good luck :]

@atassis
Copy link
Owner

atassis commented Feb 2, 2025

v0.2.1 deployed
Check that everyone is ok for you, please.
And thanks!

@felixmosh
Copy link
Contributor Author

It works as expected,
Will you recieve a PR that makes the start / stop / restart methods public.
(I need them for the rsbuild adapter, to support rsbuild reload build feature).

function runScriptPlugin(): RsbuildPlugin {
  return {
    name: 'run-script-webpack-plugin',
    setup: (api) => {
      let runServerPlugin;

      api.modifyRspackConfig((config, { isDev }) => {
        if (!isDev) {
          return;
        }

        runServerPlugin = new RunScriptWebpackPlugin({
          name: 'server.js',
          autoRestart: false,
          restartable: false,
          signal: false,
          nodeArgs: process.env.INSPECT ? ['--inspect'] : [],
        });

        config.plugins.push(runServerPlugin);
      });

      api.onCloseDevServer(() => {
        runServerPlugin?._stopServer({ force: true }); // needed since signal must be false for HMR
      });
    },
  };
}

@atassis
Copy link
Owner

atassis commented Feb 4, 2025

I won't be available to do anything this week, but you are free to create a PR, I'll check it later. Doesn't look it ld break anything, so I might approve this change

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