NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 26 forks source link

Consider turning off the import/no-dynamic-require eslint rule #2479

Open robyngit opened 2 weeks ago

robyngit commented 2 weeks ago

There are instances where dynamic requires are necessary for MetacatUI's configuration and module loading strategy. While fixing linting errors in SearchableSelectView, I had to turn off the import/no-dynamic-require rule for a specific line of code:

if (view.apiSettings && !view.semanticAPILoaded) {
  // eslint-disable-next-line import/no-dynamic-require
  require(["semanticAPI"], (_SemanticAPI) => {
    view.semanticAPILoaded = true;
    view.render();
  });
  return this;
}

The only other way to fix this eslint error would be to import the module at the top of the file. However, this is a module that's infrequently used and only needed in this specific instance, so loading it every time would unnecessarily reduce performance. These dynamic requires have the disadvantage of making it harder to track dependencies, and will make it more challenging to introduce a bundler like Webpack in the future.

This issue is to decide whether:

  1. to turn off the import/no-dynamic-require rule for the entire codebase and allow dynamic requires where necessary, or
  2. to find a way to refactor the code to avoid dynamic requires, or
  3. to continue to turn off the import/no-dynamic-require in specific instances where it's necessary, like in the example above.