feat(dynamic-import-vars): Error when no files found

[koddsson]

Rollup Plugin Name: dynamic-import-vars

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

Hey hey! I'd like the dynamic-import-vars to error if there are no files found. I think this makes sense since the function would also error on runtime since it's just a function with a switch statement that defaults to throwing a runtime error.

Example dynamic import runtime function when there are no files in the directory.

function __variableDynamicImportRuntime0__(path2) {
  switch (path2) {
    default:
      return new Promise(function(resolve, reject) {
        (typeof queueMicrotask === "function" ? queueMicrotask : setTimeout)(
          reject.bind(null, new Error("Unknown variable dynamic import: " + path2))
        );
      });
  }
}

I made this a breaking change but I can put this behind a option instead if we'd like. I think having it on by default makes a lot of sense again because this would already result in a runtime error anyways.

[shellscape]

Hey thanks for the PR. This makes sense. This only caveat is that this hasn't been an error for a very long time. Given that the Vite ecosystem uses this plugin heavily (and will for some time to come yet), I'd like to take this in a slightly different route: hide this behind an option, which is defaulted to false. That will give us time to alert the Vite maintainers (and adjacent ecosystem projects) and allow them to turn on the option and prepare for the eventual breaking change. Our next subsequent release would be to deprecate the option, and then make it the default behavior with a new major version. Kind of a PITA but it's the responsible path when there are quite literally a million people relying on a package.

[koddsson]

Hey thanks for the PR. This makes sense. This only caveat is that this hasn't been an error for a very long time. Given that the Vite ecosystem uses this plugin heavily (and will for some time to come yet), I'd like to take this in a slightly different route: hide this behind an option, which is defaulted to false. That will give us time to alert the Vite maintainers (and adjacent ecosystem projects) and allow them to turn on the option and prepare for the eventual breaking change. Our next subsequent release would be to deprecate the option, and then make it the default behavior with a new major version. Kind of a PITA but it's the responsible path when there are quite literally a million people relying on a package.

Yup that's fair enough. I've moved this to be behind a option. Let me know what you think :)

[koddsson]

Hey @shellscape; pinging you in case you don't get notifications for this. No rush though 😄

[shellscape]

Added the documentation we'd need for the new option to the README and updated the types. Something to keep in mind that we need for new features for the future. Thanks for the work on this one.

[koddsson]

Added the documentation we'd need for the new option to the README and updated the types. Something to keep in mind that we need for new features for the future. Thanks for the work on this one.

Cheers! I'll keep that in mind for next time :)