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

Feature: support trailing slash installs without tracing #339

Open
bennypowers opened this issue Jan 17, 2024 · 1 comment
Open

Feature: support trailing slash installs without tracing #339

bennypowers opened this issue Jan 17, 2024 · 1 comment

Comments

@bennypowers
Copy link

I'm working on an import map generator plugin for web-dev-server (see modernweb-dev/web#2565). We've had some success so far, but the plugin requires some unsavoury workarounds for cases which involve trailing slash mappings. Our need for trailing slash mappings is mostly for these two cases:

  1. dynamic imports by conventional path from node_modules
  2. redirecting to source modules within the current repo (or monorepo)

In the first case, we have a set of icons available under the package @patternfly/icons. The package consists of a flat set of directories, one per "icon set", each directory contains js modules, one per icon. Our icon component dynamically imports them. So for example

<pf-icon set="fab" icon="check"></pf-icon>
<script>
// above ends up doing this during it's lifecycle
const iconSet = 'fab';
const iconName = 'check';
const icon = await import(`@patternfly/icons/${iconSet}/${iconName}.js`).then(x => x.default); 
</script>

The ideal final import map for this looks something like:

{
   "imports": {
     "@patternfly/icons/": "/node_modules/@patternfly/icons/"
   }
}

However, Generator does not currently support this configuration:

{
  inputMap: {
    imports: {
     "@patternfly/icons/": "/node_modules/@patternfly/icons/"
   }
  }
}

Our workaround is to scan the filesystem and generate an imports block for the entire package. This is extra work for the end user so we're providing a function for that, but it also creates an overly large and overly specified import map for the final page:

image

Our second case is for redirecting to local repo modules. For this again, we'd prefer to issue a trailing slash resolution in the input map, but generator is unable to tolerate it:

{
   "imports": {
     "@rhds/elements/": "/elements/",
     "@rhds/elements/lib/": "/lib/"
   }
}

Once again, our solution is to walk the file system and output a big-old mapping

import { glob } from 'glob';

/**
 * Find all modules in a glob pattern, relative to the repo root, and resolve them as package paths
 * @param {string} pattern
 * @param {(spec: string) => [string, string]} fn
 */
async function resolveLocal(pattern, fn) {
  return glob(pattern, { ignore: ['**/test/**'] })
    .then(files => files.map(fn))
    .then(Object.fromEntries);
}

export default {
  plugins: [
    importMapGeneratorPlugin({
      providers: {
        '@patternfly/icons': 'nodemodules',
        '@patternfly/elements': 'nodemodules',
        '@patternfly/pfe-tools': 'nodemodules',
        '@patternfly/pfe-core': 'nodemodules',
      },
      inputMap: {
        imports: {
          ...await resolveLocal('./lib/**/*.js', spec => [`@rhds/elements/${spec}`, `./${spec}`]),
          ...await resolveLocal('./elements/**/*.js', x => [`@rhds/elements/${x.replace('elements/', '')}`, `./${x}`]),
        },
      },
    }),
  ],
};

Alternative solutions

What we've done in the past is first generate the importmap, then manually append our imports before injecting the whole thing into the page using a static site builder or some other template language. But in this case, for the web-dev-server, we'd prefer to use injectHtml, which is otherwise much more ergonomic, so that's no bueno.

Proposed solutions

From discord:

gb: having some custom rules for trailing slash mappings in input maps might well be a good option. basically they're mappings which the generator should use when doing its tracing, but not actually trace, but also not prune, which the generator usually does as well.
gb: it might even be best described as a combination of input map options -

inputMapOptions: {
  prune: false,
  iterateTrailingSlashMappings: false
}

bp: maybe

inputMapOptions: {
  '@patternfly/icons': {
    prune: false,
    iterateTrailingSlashMappings: false
  },
},
@guybedford
Copy link
Member

Firstly, thinking about this more - I think we should definitely support this as a feature without needing any custom options flags enabled.

Furthermore it should be possible to implement without any globbing process being necessary.

Just as the generator already falls back to the input map, it should be able to fall back to the trailing slash mappings to use. So that might almost be seen as a bug of sorts.

Then when outputting the final map, just like it always keeps the input map mapping "residuals", it should very much do the same for trailing slash mappings, so there is no reason they should be pruned.

It's got a few touch points, but it would just be about working through that model here I think to implement.

@bennypowers would you be interested in working on a PR further? It would certainly be great to see.

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

No branches or pull requests

2 participants