add variableName option

[ywmail]

add option variableName for another choice.

[agoldis]

@ywmail great work on PR! I will review and merge during the upcoming weekend.

[agoldis]

@ywmail
Thanks again for submitting the PR! Great work here!

1. See inline comments
2. How about adding tests that confirm that the introduced feature works?

E.g.:

  it("replaces with a value of variableName", async () => {
        global.publicPath = "customPathFromVariable";
        appendChildTrap = ({ src }) =>
          assert.strictEqual(src.split("/")[0], "customPathFromVariable");
        await compileWithWebpackVersion(
          webpackVersion,
          "variableName_pluginConf"
        );
        global.publicPath = undefined;
        delete global["publicPath"];
      }),
   it("uses default public path when variableName is undefined", async () => {
        appendChildTrap = ({ src }) =>
          assert.strictEqual(src.split("/")[0], "originalPublicPath");
        await compileWithWebpackVersion(
          webpackVersion,
          "variableName_pluginConf"
        );
      }),

[agoldis]

Curious why did you add a setter?

[ywmail]

Because webpack allow users to change the __webpack_public_path__ in their code for a dynamic outputPath, if their is no setter in property p, the application will fail with a error message:
Cannot set property p of function...
See:
https://webpack.js.org/guides/public-path/#on-the-fly
so i think it's better to add a setter to warn the user in the setter function like:

        "  set: function (value) {",
        `console.warn("You should not change __webpack_public_path__ directly, use ${PLUGIN_NAME} to config public path.")`,
        " }",

[ywmail]

Do you think it's necessary?

[agoldis]

@ywmail
Good catch, I wonder what would be the most expected behaviour for that case.
I would expect to get the value of __webpack_public_path__ if no webpack-require-from overrides exist, and if there's an override, return whatever plugin defines.
What do you think?

I will open a separate issue to gently handle this scenario meanwhile

[ywmail]

@agoldis
Yes, I also think it's better to handle this scenario in separate issue gently, a warning in setter function is just temporary in my case. I will remove this part from the PR. :)

[agoldis]

$PIX?

[ywmail]

$PIX is the global object in my project, I committed this by mistake, will remove it.

[ywmail]

Thanks for the review and your great work, it's really a great plugin. 💯
I'll update the PR later, it's my first PR on github. :) 🔢

[ywmail]

@agoldis
Update the PR: add some tests to confirm that variableName works as expected and doesn't break the original function. :)
A great thanks for reviewing the code. 💯