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

@use not working with builtin modules #148

Closed
Geekimo opened this issue Jul 28, 2021 · 8 comments
Closed

@use not working with builtin modules #148

Geekimo opened this issue Jul 28, 2021 · 8 comments

Comments

@Geekimo
Copy link

Geekimo commented Jul 28, 2021

Hello !
I just went through an issue with @use trying to load a builtin module, which starts with "sass:".

@use "sass:math";

I got the following error:

Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: Invalid Sass identifier "sass:math"
   │
 1 │ @use "../assets/scss/sass:math";
   │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Looks like the sass-resources-loader tries to locally import this module.

I'm currently using 2.2.3 through @nuxtjs/resources-loader package and hoistUseStatements is set to true.

@Geekimo
Copy link
Author

Geekimo commented Jul 28, 2021

I ran the some tests and got the following output in utils/rewritePath.js.
The file was compiled as following :

"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});
exports["default"] = rewritePaths;
exports.getRelativeUsePath = void 0;

var _path = _interopRequireDefault(require("path"));

var _logger = _interopRequireDefault(require("./logger"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { "default": obj }; }

var useRegexp = /@(import|use)\s+(?:'([^']+)'|"([^"]+)"|([^\s;]+))/g;

var getRelativeUsePath = function getRelativeUsePath(oldUsePath, absoluteUsePath, moduleContext) {
  // from node_modules
  if (/^~/.test(oldUsePath)) {
    return oldUsePath;
  }

  return _path["default"].relative(moduleContext, absoluteUsePath);
};

exports.getRelativeUsePath = getRelativeUsePath;

function rewritePaths(error, file, contents, moduleContext, callback) {
  if (error) {
    _logger["default"].debug('Resources: **not found**');

    return callback(error);
  }

  if (!/\.s[ac]ss$/i.test(file)) {
    return callback(null, contents);
  }

  var rewritten = contents.replace(useRegexp, function (entire, importer, single, _double, unquoted) {
    // Don't rewrite imports from built-ins
    if (single && single.indexOf('sass:') === 0) {
      return entire;
    }

    var oldUsePath = single || _double || unquoted;

    var absoluteUsePath = _path["default"].join(_path["default"].dirname(file), oldUsePath);

    var relUsePath = getRelativeUsePath(oldUsePath, absoluteUsePath, moduleContext);
    var newUsePath = relUsePath.split(_path["default"].sep).join('/');

    _logger["default"].debug("Resources: @".concat(importer, " of ").concat(oldUsePath, " changed to ").concat(newUsePath));

    var lastCharacter = entire[entire.length - 1];
    var quote = lastCharacter === "'" || lastCharacter === '"' ? lastCharacter : '';
    return "@".concat(importer, " ").concat(quote).concat(newUsePath).concat(quote);
  });
  callback(null, rewritten);
  return undefined;
}

I used the following snippet just before line 41 for debugging: console.log('Debug: ', { entire, importer, single, _double, unquoted });

I got the following input:

Debug:  {
  entire: '@use "sass:math"',
  importer: 'use',
  single: undefined,
  _double: 'sass:math',
  unquoted: undefined
}

The value in _double is expected to be in single isn't it ?

Regards.

@Geekimo
Copy link
Author

Geekimo commented Jul 28, 2021

Just ran another test, and I found out that due to the compilation issue, the @use with a builtin module only work if the value is single-quoted.
FYI I use Node 15.9 and Yarn 1.22.5.

@justin808
Copy link
Member

@Geekimo I'm open to a PR.

@justin808
Copy link
Member

I just shipped 2.2.4 which has a minor update.

@amannn @shokmaster @Tomburgs Any chance that you can check this issue out? I won't have time to work on a PR for a while. I'm happy to fast-track a review and ship if you guys have a PR.

And please let me know if just using single rather than double quotes is enough and that we should close this issue.

CC: @Geekimo, @mahish

@rchl
Copy link

rchl commented Aug 5, 2021

I've released new version of @nuxtjs/resources-loader with sass-resources-loader updated.

@Tomburgs
Copy link
Member

Tomburgs commented Aug 5, 2021

I believe this issue was already fixed in #147, did we just forget to push it to npm @justin808?

@justin808
Copy link
Member

Should be fixed in 2.2.4.

Thanks @Tomburgs!

@mrleblanc101
Copy link

Does not seem to work if using :export somewhere. #149

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

5 participants