Standardize mapping of browser URLs to file paths (for Resource plugins)

[thescientist13]

Discussed in #691

^{Originally posted by thescientist13 August 7, 2021}

Overview

As has come up in a couple recent issues which I believe are part a result of the fallout from #520 (changed the ordering of how default plugins run)

• strip query params from URLs #579
• fetching any JSON file that is not graph.json always returns an ES module #604
• dev server proxy is not forwarding query params #598
• exclude node modules from user workspace resolution #687
• query string parameters in node_modules (CSS) paths returns no content and 204 response #690

We have a little bit of a conundrum in this particular section of code which does the bulk of the work to resolve URLs in the browser to actual files on disk for the following (in this order)

• node_modules
• user workspace
• any custom resolvers from a Resource plugin from the user (FI runs first)

app.use(async (ctx, next) => {
  ctx.url = await resources.reduce(async (responsePromise, resource) => {
    const response = await responsePromise;
    const { url } = ctx;
    const resourceShouldResolveUrl = await resource.shouldResolve(url);
    
    return resourceShouldResolveUrl
      ? resource.resolve(url)
      : Promise.resolve(response);
  }, Promise.resolve(ctx.url));

  // bit of a hack to get these two bugs to play well together
  // https://github.com/ProjectEvergreen/greenwood/issues/598
  // https://github.com/ProjectEvergreen/greenwood/issues/604
  ctx.request.headers.originalUrl = ctx.originalUrl;

  await next();
});

Basically given the following types of URLs

• something.css?type=css
• /api/events?query=something

The issues boil down to

• query strings will break file system access / logic, eg. fs.readFile('something.css?type=css)
• If a file exists in both node_modules and user workspace, there can be incorrect results when trying to map relative resources

Considerations

Query Params

• strip query params from URLs #579
• fetching any JSON file that is not graph.json always returns an ES module #604
• dev server proxy is not forwarding query params #598

const { url } = ctx; // goes to resolvers

ctx.request.headers.originalUrl = ctx.originalUrl; # stashed in a header for plugins - https://github.com/ProjectEvergreen/greenwood/blob/v0.15.0/packages/plugin-import-css/src/index.js#L22

Because of needing to support query params for API calls or appending hints for non-standard ESM, we have two options

1. Force every resolver to use barePathUrl to strip query params
2. Strip it at the Koa level before handing to resolvers and still keep in a secret header (current)

Although solution 2 has been working, it's a bit of a "hidden" API, so not sure if there is enough docs for it, or a better way to handle this. But it avoid downstream plugins from breaking because of the file reading issue.

Resolvers

• exclude node modules from user workspace resolution #687
• query string parameters in node_modules (CSS) paths returns no content and 204 response #690

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);
    
    return resourceShouldResolveUrl
      ? resource.resolve(response)
      : Promise.resolve(response);
  }, Promise.resolve(ctx.url));

There is a referenced issue to just forward the resolved URL, which seems fine for now. But there is another issue that to support relative URLs, which helps with things like #689 (comment), we have to make sure that (for example), user workspace resolver ONLY looks in the user workspaceinstead of trying to a node_modules path and reducing it down to something that might actually be in the user's workspace, by coincidence.

Not sure if we need to something like a negative resolve check to make sure user workspace isn't going to inadvertently resolve to some other legitimate resource? Otherwise, we can't always deny all possibilities, like we can since we know about node_modules