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

Sass imports from node_modules #132

Open
noahtallen opened this issue May 26, 2021 · 1 comment
Open

Sass imports from node_modules #132

noahtallen opened this issue May 26, 2021 · 1 comment

Comments

@noahtallen
Copy link

Howdy! In sass, you can use sass-loader with Webpack to resolve Sass imports from node modules. This can look something like the following:

@import 'npm-package/stylesheet'

// etc

This should resolve to node_modules/npm-package/stylesheet.scss. Unfortunately, cabinet + sass-lookup doesn't quite handle this use case. You can get it to resolve using just cabinet, by passing node_modules as a directory, but dependency tree doesn't accept multiple directories in general, so that won't work for our use case. I was wondering what a good path would be towards implementing support.

A really naive implementation could be something like this as the lookup function for sass in cabinet.

function sassResolver(options) {
  let { directory } = options;
  if (typeof directory === 'string') {
    directory = [ directory ];
  }
  directory.push( 'node_modules' );
  return sassLookup({ ...options, directory })
}

This does work, but it wouldn't be able to handle different node_modules locations, such as in monorepos. One could also add a custom resolver like this: https://github.com/Automattic/wp-calypso/blob/6ef9f9278aaf368cd629ba69c33c4768dc7ea08d/bin/render-stylesheet.js#L16-L23. But I'm not sure where exactly to put that -- would the sass-lookup package be a good fit?

What are your thoughts?

@mrjoelkemp
Copy link
Collaborator

Hey @noahtallen! Thanks for the question and context. Seems like we could overhaul https://github.com/dependents/node-sass-lookup/blob/master/index.js and incorporate sass-loader or replace sass-lookup entirely with sass-loader (which is probably more to your suggestion). I'm open to either that satisfies existing tests and also passes new tests associated with your imports case above.

Happy to review a PR. Thanks again.

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