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

Yarn pnp cannot resolve ~ imports #168

Open
simPod opened this issue Jul 3, 2022 · 7 comments
Open

Yarn pnp cannot resolve ~ imports #168

simPod opened this issue Jul 3, 2022 · 7 comments
Labels
help wanted Looking for help to reproduce, debug, or open a PR

Comments

@simPod
Copy link

simPod commented Jul 3, 2022

Expected Behavior

Type defs to be generated

Current Behavior

Can't find stylesheet to import.

when using @import ~...

Possible Solution

?

Steps to Reproduce (for bugs)

I'm using yarn pnp and this config

export const config = {
  aliasPrefixes: {
    '.~': 'node_modules/',
    '~': 'node_modules/',
  },
  exportType: 'default',
  implementation: 'sass',
  logLevel: 'error',
};
@import "~bootstrap/scss/card";

.arule {
  flex: 1;
}

Context

However, with yarn pnp there are no node_modules.

Your Environment

  • Version used: 6.5
  • Yarn: 3.2
@skovy
Copy link
Owner

skovy commented Jul 24, 2022

I'm not familiar with yarn pnp, but open to accepting contributions if it's straightforward and makes sense to include in the tool.

@skovy skovy added the help wanted Looking for help to reproduce, debug, or open a PR label Jul 24, 2022
@skovy
Copy link
Owner

skovy commented Aug 5, 2023

Any more details? A reproduction repo would be helpful. Does this work outside of this tool? This seems like a general issue with potential with SASS and Yarn PNP. Will close soon as unclear how to move forward with the current information.

@skovy skovy added the question Further information is requested label Aug 5, 2023
@GeorgeTaveras1231
Copy link

GeorgeTaveras1231 commented Sep 7, 2023

@skovy the issue is that aliasPrefixes is limited to mapping a string to a static folder, and it does not use the standard resolution algorithms implemented by require.resolve or other npm packages such as resolve, and enhance-resolve.

It would be good to make the resolver use enhance-resolve (or allow customizing it to do so). since the ~ convention was (AFAIK) established by webpack, and webpack uses enhance-resolve for its resolution. It also provides a lot of flexibility to resolve sass modules distributed as npm packages (such as searching for the sass field in package.json, and the sass condition in the package.json's export field.

I implemented this once before, for an internal project. I'll link the implementation here in case it is helpful

https://gist.github.com/GeorgeTaveras1231/8e18ef6eba9495f077ce03e94a00e333


Update:

I resolved the issue on my repo by using the importer implementation I shared above and using the importer config in typed-scss-modules.config.js

@skovy
Copy link
Owner

skovy commented Sep 16, 2023

Thanks, @GeorgeTaveras1231 for the details, that's really helpful!

I would be open to a more robust, standard resolution strategy using the same approach other tools use. Ideally, we'd use node APIs or well-established packages to avoid needing to reimplement this.

I have no plans of working on this, but I'd be open to this contribution from anyone.

@skovy skovy removed the question Further information is requested label Sep 16, 2023
@markormesher
Copy link

@GeorgeTaveras1231 have you considered publishing your resolver plugin as a tiny NPM package that could be plugged into TSM? Alternatively @skovy would you consider enhanced-resolve as well-established enough to accept a PR that uses it to add the functionality directly? If so I'd be happy to work on it.

@skovy
Copy link
Owner

skovy commented Oct 9, 2023

@markormesher that seems well-established. Are there any downsides, eg: breaking changes we'd need to make, or do we need to expose more configuration?

If it's a simple drop-in and all existing use cases continue to work, and it magically fixes other use cases (like Yarn PNP) then I'm open to it. I'm not as familiar with this and the implications so want to approach this with caution.

@markormesher
Copy link

It shouldn't break any existing use cases, but we'd obviously want to test that. We could expose an extra optional config to let people control enhanced-resolve's behaviour, but the default should work out of the box.

I'll take a look at this in a fork when I get a chance and open a PR when I have something worth reviewing and discussing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Looking for help to reproduce, debug, or open a PR
Projects
None yet
Development

No branches or pull requests

4 participants