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

No support for subpath patterns? #289

Closed
dschnare opened this issue Jan 21, 2024 · 14 comments · Fixed by #290
Closed

No support for subpath patterns? #289

dschnare opened this issue Jan 21, 2024 · 14 comments · Fixed by #290

Comments

@dschnare
Copy link

Esmock seems to have trouble resolving modules from packages with "exports" that use subpath patterns.

Some package (my-pack)

{
  "exports": {
    "./*": {
     "default": "./src/*/index.js",
     "types": "./types/*/index.d.ts"
    }
  }
}

Then in another package for example.

import { Something } from 'my-pack/stuff' // this line fails due to module ID not being found
@dschnare
Copy link
Author

@dschnare
Copy link
Author

Interestingly, this error only occurs for me for some subpaths and not others from a package. To workaround I had to use the pattern to catch most subpaths then have an explicit export for subpaths that cause the error.

{
  "exports": {
    "./*": {
     "default": "./src/*/index.js",
     "types": "./types/*/index.d.ts"
    },
    "./my-path": {
      "default": "./src/my-path/index.js",
      "types": "./src/my-path/index.d.ts"
    }
  }
}

@iambumblehead
Copy link
Owner

iambumblehead commented Jan 22, 2024

@dschnare subpaths are supported and if one is not resolved correctly that's a bug. esmock uses resolvewithplus to resolve module ids, so that's where the solution should happen.

Node's dsl esm pattern language is tricky to parse. For resolvewithplus, official node examples were copied to unit-tests used by resolvewithplus and when missing cases have been discovered they've been resolved in new versions of resolvewithplus

@iambumblehead
Copy link
Owner

@dschnare I have not seen wildcard export namespace names before, eg {"exports":{"./namepace*": { ... }}} that usage does not appear to be documented at the link you shared.

@dschnare
Copy link
Author

dschnare commented Jan 22, 2024 via email

@iambumblehead
Copy link
Owner

@dschnare thanks for reporting. Its always good to learn about these cases I didn't know about.

@iambumblehead
Copy link
Owner

@dschnare is this what you think the exports, directory and file layout should look like for unit-testing this?

{
  "name": "asterisk-dir-name",
  "type": "module",
  "exports": {
    "./*": {
      "default": "./src/*/index.js",
      "types": "./types/*/index.d.ts"
    }
  }
}
.
├── package.json
├── src
│   └── mystuff
│       └── index.js
└── types
    └── mystuff
        └── index.d.ts

@iambumblehead
Copy link
Owner

@dschnare I wonder specifically how the named-property asterisk and then its nested, matched value must correspond to each other and the specifier used to import such a module.

For example, should the outer "./*" return any directory or only a directory that corresponds with the imported moduleId?

Maybe we should first implement any basic solution and then refine that later as any edge-cases are reported to fail.

@iambumblehead
Copy link
Owner

@dschnare
Copy link
Author

That test scenario is perfect. That's what we have currently in our project. Some folders work, while others didn't so I had to specifically add an export for that folder.

@dschnare
Copy link
Author

dschnare commented Jan 23, 2024

@iambumblehead It looks like from the documentation that the subpath pattern ./* would match any path like "mymodule/something", "mymodule/something/cool", etc. And technically * could be included multiple times in the export pattern. This I didn't know, but interesting.

All instances of * on the right hand side will then be replaced with this value, including if it contains any / separators.

@iambumblehead
Copy link
Owner

@dschnare the PR was just updated so that the tests will pass in windows. Feel free to recommend any changes

@dschnare
Copy link
Author

@iambumblehead I understood that incorrectly. The * can appear multiple times on the right side. On the left side it appears it should only occur once.

@iambumblehead
Copy link
Owner

if the overall approach is okay (as it seems to be) its good for me if the PR is accepted. Later on, when needed, the place that expands the asterisks on the right side could be improved for supporting more situations.

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