linkedin / eyeglass

NPM Modules for Sass
741 stars 60 forks source link

Unable to import eyeglass module from included package #171

Closed ThePeach closed 7 years ago

ThePeach commented 7 years ago

this might have been answered somewhere else, but I couldn't find any pointers.

My project X is importing an npm module Y that uses several eyeglass modules. Project X has got a sass file that imports some of the eyeglass modules that are in the npm module Y. Project X has got eyeglass as a dependency in package.json but this results in the error Error: Error: Could not import modularscale from any of the following locations.

jackw commented 7 years ago

Without more info (e.g. names of modules, structure of module Y that uses several eyeglass modules) it could be tricky for anyone to track down the issue. Only suggestion I can give based on info provided is to check is if module Y is also a dependency and is listed in Project X package.json along with eyeglass.

If that doesn't fix things please can provide more info and I'm sure someone will be able to assist.

ThePeach commented 7 years ago

sorry for the lack of information @jackw but didn't have direct access to the problem. Now I do, following the full description of the issue:

So Project X, @namespace/patternlab-base-toolkit has a namespaced npm module Y @namespace/submodule in its dependencies, alongside eyeglass:

"dependencies": {
  "browser-sync": "^2.0.0",
  "chalk": "^1.1.3",
  "eyeglass": "^1.2.1",
  "gulp": "gulpjs/gulp#4.0",
  "gulp-sass": "^3.1.0",
  "gulp-stylelint": "^3.9.0",
  "minimist": "^1.2.0",
  "patternlab-node": "^2.0.0",
  "styleguidekit-assets-default": "^3.0.0",
  "styleguidekit-mustache-default": "^3.0.0",
  "stylelint-config-standard": "^16.0.0",
  "@namespace/submodule": "git@xxxxx"
},

Module @namespace/submodule has got the following modules as dependencies:

"dependencies": {
  "modularscale-sass": "^3.0.2",
  "normalize-scss": "^7.0.0",
  "typi": "^3.1.0"
}

when running npm install I get:

namespace/patternlab-base-toolkit@1.0.0 /Users/peach/repos/patternlab-base-toolkit
└─┬ @namespace/submodule@1.0.0  (git+ssh://xxxxx)
  ├── modularscale-sass@3.0.2
  ├── normalize-scss@7.0.0
  └── typi@3.1.0

in the style.scss of project X, I have @import "modularscale"; and when I run the sass task from gulp I get:

Error in plugin 'sass'
Message:
    source/sass/style.scss
Error: Error: Could not import modularscale from any of the following locations:
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale.scss
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale.sass
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale.css
         /Users/peach/repos/patternlab-base-toolkit/source/sass/_modularscale.scss
         /Users/peach/repos/patternlab-base-toolkit/source/sass/_modularscale.sass
         /Users/peach/repos/patternlab-base-toolkit/source/sass/_modularscale.css
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale/index.scss
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale/index.sass
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale/index.css
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale/_index.scss
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale/_index.sass
         /Users/peach/repos/patternlab-base-toolkit/source/sass/modularscale/_index.css
        on line 10 of source/sass/style.scss
>> @import "modularscale";
   --------^

The gulp sass task is the following:

gulp.task('pl-sass', function () {
  const sassOptions = {
  };

  return gulp.src(path.resolve(paths().source.sass, '**/*.scss'))
    .pipe(sass(eyeglass(sassOptions)).on('error', sass.logError))
    .pipe(gulp.dest(path.resolve(paths().source.css)))
})

let me know if you need any other output/code snippet.

ThePeach commented 7 years ago

ok, so I've found two problems here: the first one is a bug with modularscale-sass#141, which I managed to fix manually by modifying its package.json. The second error instead seems to be more related to eyeglass than anything else: which means that only if I include the module in the package.json of the main project, eyeglass will pick it up, otherwise nope :( which is a real shame.

Any workaround for this or adding it to the package.json is the only way forward?

jackw commented 7 years ago

The first problem can be addressed easily as you discovered by editing the modularscale package.json file.

I believe (I'm sure someone will correct me if I'm wrong) the second issue you describe is by design.

Can I ask why you don't want to install the module with the --save-dev flag to add it to your projects package.json? It means anyone else working on the project will have an easier time resolving dependencies.

ThePeach commented 7 years ago

Thanks @jackw, to answer your question the simple reason is because the way the main core project works is by importing and copying the sass files over and relying on the dependencies of the submodules. I can rework the way the whole thing is set up as I'm currently playing around it, but it would have been nicer to have independent modules plugged in, mostly because I thought eyeglass could sniff the right modules whether they have been installed directly or by an indirect dependency, if you see what I mean.

eoneill commented 7 years ago

@jackw is correct in that this is an intentional design decision in eyeglass. We do not allow transitive dependencies to be imported. Your dependencies have to publicly expose anything they intend to be consumed by their dependents.

For example, if you intend for modularscale to be a public export of @namespace/submodule, you could do something like this...

// @namespace/submodule/sassDir/modularscale/index.scss
@import "modularscale"; // imports (and exposes) modularscale-sass

Then your consuming app would do the following...

// app/styles.scss
@import "@namespace/submodule/modularscale";

Additionally, The following modules are incompatible with eyeglass message is just a warning and won't actually affect your build (unless there are true incompatibility issues in the code, which is unlikely).

ThePeach commented 7 years ago

thanks @eoneill, I think I've missed something in the initial description on how the main base project X works: on top of having module Y as a dependency, it will also copy all of its assets (templates) and SCSS files into its own directory structure. This is something that actually works as is and it's kind of necessary for the whole project to work as expected.

As you can see this is clearly a very specific setup, so the solution you're proposing might not actually work.

Since my basic idea is to keep anyway things as much separated as possible, (as, I don't really care the SCSS files are being copied, I just want to use them as is), I was thinking of making the submodule Y an eyeglass module and solve the aforementioned problems of the eyeglass dependencies of Y this way.

I'll try to give it a go, but if you have any other idea, on how to get around this without having to do more, let me know, any hint is greatly appreciated :)

eoneill commented 7 years ago

My suggestion would be to declare explicit dependencies on the things you need.

That is, if your top-level app/module and your submodule both need modularscale, they should both declare a dependency on modularscale.

This is, in general, a best practice and will help you avoid issues and unexpected side-effects down the road. With potentially deeply nested dependency graphs, implicitly exposing transitive dependencies is a bad idea :)

ThePeach commented 7 years ago

@eoneill so I've made my submodule an eyeglass module: this way I could avoid the hassle of working out a way around of using any eyeglass dependencies the submodule has, and all I have to do now is @import "@namespace/submodule" in the main project X, which works just fine.

This is not exactly what I wanted as it restricts the ability of using any of the submodule components directly, excluding others. It's not great, but it's not even that bad.

So far, thank you very much for the help.

I guess this can now be closed.

eoneill commented 7 years ago

Glad you figured out a way that mostly works for you. Let us know if you run into any other issues.