Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Asyncronous functions for config.url and config.import #1236

Closed
DavidArchibald opened this issue Dec 2, 2020 · 1 comment · Fixed by #1277
Closed

Support Asyncronous functions for config.url and config.import #1236

DavidArchibald opened this issue Dec 2, 2020 · 1 comment · Fixed by #1277

Comments

@DavidArchibald
Copy link
Contributor

I don't believe these details should effect replication but I've included them just in case:

  • Operating System: Windows
  • Node Version: v10.19.0
  • NPM Version: 6.14.0
  • webpack Version: 4
  • css-loader Version: 5.0.0

Expected Behavior / Situation

I expected to be able to use asynchronous code for filtering URLs and imports, for example:

{
  loader: "css-loader",
  options: {
    url: async (url, contextPath) => ...
  }
}

The specific use case I'm running into is some Webpack utilities that rely on callbacks like compiler.inputFileSystem.stat which can easily be turned into a Promise and then awaited. I'm using complex file system searching due to a custom resolver I'm developing and thusly its interface with css-loader. Disregarding the specific and looking into the general, I don't see this as an adverse design choice to make since a lot of code can be asynchronous for a lot of reasons. It's probably this way because filtering was assumed to be relatively simple and blocking.

Actual Behavior / Situation

The asynchronous functions aren't awaited and so behave the same as url: true because any promise is truthy.

Modification Proposal

It turns out the loader is already an asynchronous function so this isn't really much of a change. Here's where I believe the filtering is done for options.import:

filter: getFilter(options.import, this.resourcePath),

Here's the one for options.url:

filter: getFilter(options.url, this.resourcePath),

I believe to solve this would be as trivial as prepending the function call with await, i.e. doing await getFilter(...) because in the case of non-promises Node just returns the result as per normal, e.g. await () => false and others are safe pieces of code. I looked for the specification backing this but apparently this second hand source StackOverflow answer may be the remains now that the proposal text goes to a deadlink on the proposal's Github.

Stylistically these specifics doesn't really matter to me but due to what I believe to be a literally 12 character addition I didn't open a PR directly because style concerns or just conversation around this could easily dwarf the actual contribution size significantly and require being redone entirely. I'd be happy to open a PR if that would be helpful of course.

@alexander-akait
Copy link
Member

Feel free to send a PR

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 a pull request may close this issue.

2 participants