query string parameters in node_modules (CSS) paths returns no content and 204 response

[thescientist13]

Type of Change

• New Feature Request
• Documentation / Website
• Improvement / Suggestion
• Bug
• Other (please clarify below)

Summary

As part of testing development of theme packs (#689) it was observed that when a CSS file was referenced via an import, e.g.

import themeCss from '/node_modules/<package>/dist/theme.css?type=css';

url /node_modules/greenwood-starter-presentation/dist/styles/theme.css?type=css

  TypeError: Cannot read property 'replace' of undefined
      at /Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/plugin-import-css/src/index.js:33:50
      at new Promise (<anonymous>)
      at ImportCssResource.intercept (/Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/plugin-import-css/src/index.js:31:12)
      at /Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:106:52
      at processTicksAndRejections (internal/process/task_queues.js:93:5)
      at async /Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:97:24
      at async /Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:97:24
      at async /Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:96:29
      at async /Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:86:5
      at async /Users/owenbuckley/Workspace/github/repos/greenwood-starter-presentation/node_modules/@greenwood/cli/src/lifecycles/serve.js:49:5

In this case, The URL being passed to plugin-standard-css was still including the query string and thus failing the extension check, since path.extname was also including the query string .css?type=css and not just .css, and thus failing the test;

Details

This dovetails a little more into a discussion I opened to get a better handling on handling URLs and filesystem access, so will defer to that and so will just try and land a simple patch for this for now that tries to address the resolver issue detailed in that discussion

After Greenwood resolvers go first, the results of those never get passed to next plugins, because we always pass the original URL to each resolvers, instead of saying forwarding the results of the node_modules resolver to any other resolvers that may want to reuse that logic.

  ctx.url = await resources.reduce(async (responsePromise, resource) => {
    const response = await responsePromise;
    // const { url } = ctx; just keep using the resolved response
    const resourceShouldResolveUrl = await resource.shouldResolve(response);  // passing response here instead
    
    return resourceShouldResolveUrl
      ? resource.resolve(response) // passing response here instead
      : Promise.resolve(response);
  }, Promise.resolve(ctx.url));

Thus, in the above situation, with the existing code, the query string was always getting passed to plugin-standard-css and it wasn't able to handle that since we've never tested it with node_modules paths + query strings.

[thescientist13]

I don't think this is a regression as a result of the ordering of plugins changing in #520 but will look into it as part of the PR.