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 import with webpack 5 #87

Closed
nici9797 opened this issue Oct 22, 2021 · 18 comments
Closed

sass import with webpack 5 #87

nici9797 opened this issue Oct 22, 2021 · 18 comments

Comments

@nici9797
Copy link

nici9797 commented Oct 22, 2021

I'm using webpack 5 and bootstrap and tried to import the sass files as documented.
import 'path/to/node_modules/vanillajs-datepicker/sass/datepicker';
import 'path/to/node_modules/vanillajs-datepicker/sass/datepicker-bs4';

When I build the project, I get these errors:
ERROR in ./src/main.js
Module not found: Error: Package path ./sass/datepicker is not exported from package \node_modules\vanillajs-datepicker (see exports field in \node_modules\vanillajs-datepicker\package.json)

ERROR in ./src/main.js
Module not found: Error: Package path ./sass/datepicker-bs4 is not exported from package \node_modules\vanillajs-datepicker (see exports field in \node_modules\vanillajs-datepicker\package.json)

Found a workaround to adjust the exports in the package.json file from the datepicker:
"exports": {
".": "./js/main.js",
"./Datepicker": "./js/Datepicker.js",
"./DateRangePicker": "./js/DateRangePicker.js",
"./locales/": "./js/i18n/locales/.js",
"./locales/": "./js/i18n/locales/",
"./sass/": "./sass/.scss",
},

Can you please check this ?

@mymth
Copy link
Owner

mymth commented Nov 1, 2021

It looks like you misunderstand. The "path/to/nodo_modules" part has to be replaced with the absolute or the relative (from your stylesheet) path to your project's node_modules folder.

Anyway, I believe making package.json's exports paths available in sass is WebPack's trick and you shouldn't consider sass can read package.json.

When you want to use @import path starting with package name, I think the proper way to do it is adding the the node_modules folder's path to the includePaths sass option in the sass-loader options of your webpack.config.js.

// modules.rules[]
{
  // ...
  use: [
    // ...
    {
      loader: "sass-loader",
      options: {
        // ...
        sassOptions: {
          includePaths: [
            path.resolve(__dirname, './node_modules'),
          ],
        },
      },
    },
    // ...
  ],
  //...
},
// ...

I actually have never used WebPack in actual development. So, it would be appreciated if someone corrects me if I said something wrong.

@Matu2083
Copy link

I am actually not sure if that's true. I have also tried the above solution, however, the described error persists. At least with Webpack 5 exports specified in the module's package.json are explicitly respected (see here) and will result in an error as described above.

I was however able to access the css-sources after manually "./css/": "./dist/css/.css" to the package.json (as initially suggested).
For the sass sources there also seems to be an issue with putting the @import "datepicker" statement at the end of the datepicker-bs4.scss file as certain variables are not defined otherwise.
After moving the import statement upfront and adding "./sass/": "./sass/*.scss" to the package.json the Webpack 5 build was fine. A better solution would probably be to put all variables into a central variables file and load that at the top of each individual sass file.
Note that there was also no need to specify the sassOptions.

Furthermore, see this quite related issue, where I also got the first link from.
Happy to provide a PR in the upcoming days.

@mymth
Copy link
Owner

mymth commented Nov 22, 2021

@Matu2083, I'm not saying Webpack doesn't allow users to load sass/css files using the exports field's paths. What I meant is, instead of relying on Webpack's hack, setting up sass through webpack.config would be more preferable because it allows you to use the same technique when you use gulp, rollup, or even Sass CLI, and I'd like to encourage people to do so.

The example in my comment is for using @import "vanillajs-datepicker/sass/datepicker"; in your app's sass style sheet. It does nothing to the .scss file path in the import statement in .js file. The error message in @nici9797's comment suggests that you guys tried to import the datepicker's sass style sheet in your app's main.js. If you change the place to import the scss file from the .js file to your app's sass style sheet, my example should work to solve the error.

It looks like the method to import you used is the one meant for importing .css file. (this may be a good example.)
Since I'm not familiar with Webpack, I tested what would happen with it and found that the file in each import statement is compiled separately and injected into the HTML as an individual <style> tag. And for this reason, to override the module's variables in the app's style sheet in order to customize module's styles cannot be used with this method.
(I'm actually not sure if "compiled separately" is correct but thinking so makes sense to me because the way sass files are imported somehow looks to me like they were loaded as ES module and ES modules are isolated from each other.)

The issue with the location of @import "datepicker" is probably because of it. (I assume you wrote 2 import statements for Bootstrap and datepicker in your main.js.) Since datepicker-bs4.scss uses Bootstrap's variables, its compile process needs to access their definitions. However, since Bootstrap's compile is processed separately, it cannot do it and results in the error.

The reason why the error was gone by changing the location is because all the variable declarations in datepicker-bs4.scss have the !default flag. Since those variables are also defined in datepicker.scss without using external variables, if datepicker.scss is imported at the beginning, the compile process ignores datepicker-bs4's variable declarations and doesn't try to access Bootstrap's variables.
Also, the dp-button mixin, which applies framework's button style to datepicker's buttons, is never used if datepicker.scss is imported at the beginning. There's one place in datepicker.scss that chicks the existence of the mixin and calls it if it exists. It's the only place where the mixin is called. And the mixin is defined after the check runs while importing datepicker.scss. As a result, the mixin is never called.
In consequence, the output is no different from datepicker.scss's styles. I think your datepicker should look the same as the Plain CSS demo's instead of the Bootstrap demo's. (You'll' be able to see obvious difference if you enable the clear or today button.).

For the above reasons, @import "datepicker" actually must be done at the end. And to make it possible, both Bootstrap's and the datepicker's styles need to be imported in sass file so that Sass can process everything in one process.

Now, let's think about the validity of having the sass folder's path in the exports field. To me, it looks like the issues you had and your response to them, which I think inappropriate, were caused by mixing up the functionality of Sass's @import and JavaScript's import statements. Since the exports field is meant for being used by JavaScript's import statement/function, if the sass folder path is in there, it may give users a false impression that this library's .scss files can be used by importing in their .js file. And it can lead them to the same issues here. I don't want it to happen.

So, in conclusion, unless the use of package.json within Sass becomes a (at least de facto) standard, I will not add the sass folder's path to the exports field.
However, if reasonable reasons are provided, I may change my mind. If you or someone has one, please comment here. I'll be happy for learning something new.

@mymth
Copy link
Owner

mymth commented Nov 22, 2021

For your reference, here's the code I was able to create the dist file (dist/main.js) successfully without modifying the library's source during the test.

Structure:

├─ src/ 
│   ├─ index.js
│   └─ app.scss
├─ index.html
├─ package.json
└─ webpack.config.js

src/index.js:

import 'bootstrap';
import {Datepicker} from 'vanillajs-datepicker';

import './app.scss';

const el = document.querySelector('.date');
var datepicker = new Datepicker(el, {
  buttonClass: 'btn',
  clearBtn: true,
  todayBtn: true,
});

src/app.scss:

@import "bootstrap/scss/bootstrap";

$dp-dropdown-z: 9999;

@import "vanillajs-datepicker/sass/datepicker-bs4";

$body-color: red;

body {
  color: $body-color;
}

index.html:

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <title></title>
</head>
<body>
  <div class="container mt-3">
    <div class="form-group"><input type="text" class="form-control date"></div>
  </div>
  <script type="text/javascript" src="./dist/main.js"></script>
</body>
</html>

package.json:

{
  "dependencies": {
    "bootstrap": "^4.6.1",
    "css-loader": "^6.5.0",
    "sass": "^1.43.4",
    "sass-loader": "^12.3.0",
    "style-loader": "^3.3.1",
    "vanillajs-datepicker": "^1.1.4",
    "webpack-cli": "^4.9.1"
  },
  "scripts": {
    "build": "webpack-cli"
  }
}

webpack.config.js:

const path = require('path');

module.exports = {
  mode: 'development',
  entry: './src',
  module: {
    rules: [
      {
        test: /\.s[ac]ss$/i,
        use: [
          "style-loader",
          "css-loader",
          {
            loader: "sass-loader",
            options: {
              implementation: require("sass"),
              sassOptions: {
                includePaths: [path.resolve(__dirname, './node_modules')],
              }
            },
          },
        ],
      },
    ],
  },
};

@Matu2083
Copy link

@mymth: Thank you for your detailed response. I can confirm that your above setup works. After giving it some thought I agree with your points referring to the sass folder. I think an open question is whether you want to make the dist/css available for export.

@tagliala
Copy link

tagliala commented Feb 27, 2022

Hi, thanks for this detailed explanation

So, in conclusion, unless the use of package.json within Sass becomes a (at least de facto) standard, I will not add the sass folder's path to the exports field.

It is not a de-facto standard, webpack supports some extra conditions described at
https://webpack.js.org/guides/package-exports/#conditions, including sass

As a non-javascript developer, I do not understand the side effects of having something like

    "./sass/*": {
      "sass": "./sass/*"
    }

Since sass is not supported by other asset bundlers, I guess it will be simply ignored. Is this a viable workaround for the issue?

@matewka
Copy link

matewka commented Mar 4, 2022

@tagliala yes! It seems to be the right way to go.

But from my experience, this approach simply DOESN'T WORK. The documentation mentions sass condition in the table but below the table, where all conditions are described in detail sass is missing. Maybe it's a flawed documentation?

@tagliala
Copy link

tagliala commented Mar 4, 2022

Maybe it's a flawed documentation?

I don't know. I've tested it and it works with the webpack configuration provided by webshakapacker

You can give a try to this branch: https://github.com/tagliala/vanillajs-datepicker/tree/bugfix/export-sass

@matewka
Copy link

matewka commented Mar 4, 2022

You're right @tagliala . It worked.

I wonder what might be the problem on my side. Do you think there's anything else I need to put in my package.json besides exports?

@tagliala
Copy link

tagliala commented Mar 4, 2022

I wonder what might be the problem on my side. Do you think there's anything else I need to put in my package.json besides exports?

Sorry, I didn't get this, could you please clarify?

@matewka
Copy link

matewka commented Mar 4, 2022

@tagliala let's forget about it. I figured out my problem. Had to tweak the exports configuration.

@tindl88
Copy link

tindl88 commented Mar 16, 2022

I don't import SCSS via @import.
Like @nici9797 mentioned. the issue still exist even add includePaths in sassOption.
Screen Shot 2022-03-16 at 17 09 06

@tagliala
Copy link

@tindl88 does this branch work for your use case? https://github.com/tagliala/vanillajs-datepicker/tree/bugfix/export-sass

@tindl88
Copy link

tindl88 commented Mar 17, 2022

@tindl88 does this branch work for your use case? https://github.com/tagliala/vanillajs-datepicker/tree/bugfix/export-sass

Thanks @tagliala, but I still see that error with that branch.

package.json
...
"vanillajs-datepicker": "https://github.com/tagliala/vanillajs-datepicker.git#bugfix/export-sass"

@ziorufus
Copy link

I have the same problem, but @mymth code isn't applicable in my case since I'm using vue-cli (instead of webpack). For now, I removed the exports section in the package.json, but I know this is not a solution. Anyone can help me?

@mymth
Copy link
Owner

mymth commented Apr 17, 2022

@ziorufus It looks like vue-cli uses webpack internally. So, adding the same sass-loader options to your vue.config.js like below seems to work.

const path = require('path');
const { defineConfig } = require('@vue/cli-service')
module.exports = defineConfig({
  transpileDependencies: true,
  css: {
    loaderOptions: {
      sass: {
        sassOptions: {
          includePaths: [
            path.resolve(__dirname, './node_modules'),
          ],
        },
      },
    },
  },
});

Just in case you are unaware of this: the above is to import datepicker's scss file inside stylesheet like this:

<style lang="scss">
  @import 'vanillajs-datepicker/sass/datepicker';
</style>

not like this:

<script setup>
  import from 'vanillajs-datepicker/sass/datepicker.scss';
</script>

@ziorufus
Copy link

@mymth thank you very much, it works perfectly!

@fromcouch
Copy link

I had the exact same problem. I can't import in my Laravel Project like this way in my app.scss:

@import 'vanillajs-datepicker/sass/datepicker';

Finally I found if I use the full path work as expected

@import '/node_modules/vanillajs-datepicker/sass/datepicker';

I hope helps

@mymth mymth closed this as completed in d94a581 Feb 12, 2023
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

8 participants