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 1.63.0 cannot be imported as a module #1995

Closed
TheLonelyAdventurer opened this issue Jun 7, 2023 · 12 comments · Fixed by #2006
Closed

SASS 1.63.0 cannot be imported as a module #1995

TheLonelyAdventurer opened this issue Jun 7, 2023 · 12 comments · Fixed by #2006
Assignees
Labels

Comments

@TheLonelyAdventurer
Copy link

SASS Version: 1.63.0
Node Version: 18.16.0

Steps to reproduce:

Package.json:

{
  "name": "minimal",
  "version": "1.0.0",
  "description": "",
  "main": "test.js",
  "type": "module",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "sass": "^1.63.0"
  }
}

test.js:

import sass from 'sass'; 

Expected

No errors.

Log

(node:3040) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
C:\***********\minimal\node_modules\sass\sass.node.js:1
import * as util from "util"
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at internalCompileFunction (node:internal/vm:73:18)
    at wrapSafe (node:internal/modules/cjs/loader:1176:20)
    at Module._compile (node:internal/modules/cjs/loader:1218:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Node.js v18.16.0

Notes

The same import works with 1.62.0

@duan602728596
Copy link

duan602728596 commented Jun 8, 2023

Same question. Should output esm's files as .mjs, not .js. If there is no "type": "module", the .js file still uses commonjs.

In the esm environment, can use this method to load the module first:

import { createRequire } from 'node:module';

const require = createRequire(import.meta.url); 
const sass = require('sass');

@nex3 nex3 self-assigned this Jun 8, 2023
@nex3 nex3 added the bug label Jun 8, 2023
@nex3
Copy link
Contributor

nex3 commented Jun 8, 2023

Thanks for bearing with us, folks. We changed our release infrastructure pretty significantly as part of adding support for running in the browser, and this is an unintended side effect. Expect a fix some time tomorrow afternoon PST.

@nex3
Copy link
Contributor

nex3 commented Jun 9, 2023

It took a bit more time than expected to square the circle of browser and Node.js dependencies, but once we land google/dart_cli_pkg#131 and #2006, we should be able to release a fix for this!

@webketje
Copy link

webketje commented Jun 9, 2023

Confirmed here too.. There are 2 issues:
The first is that package.json exports contains only default and require, yet the package.json is not type: module. So basically, Node will assume that files ending on .js are commonjs (they are not).

In the generated dist file sass.dart.js around line 53, there is a direct require statement that does not check whether it is actually available:

if (dartNodeIsActuallyNode) {
  // This line is to:
  // 1) Prevent Webpack from bundling.
  // 2) In Webpack on Node.js, make sure we're using the native Node.js require, which is available via __non_webpack_require__
  // https://github.com/mbullington/node_preamble.dart/issues/18#issuecomment-527305561
  var url = ("undefined" !== typeof __webpack_require__ ? __non_webpack_require__ : require)("url");

I reran after adding the package.json type: module and added the following to sass.dart.js and it worked:

if (dartNodeIsActuallyNode) {
  // This line is to:
  // 1) Prevent Webpack from bundling.
  // 2) In Webpack on Node.js, make sure we're using the native Node.js require, which is available via __non_webpack_require__
  // https://github.com/mbullington/node_preamble.dart/issues/18#issuecomment-527305561
  var url
  if (typeof __webpack_require__ !== 'undefined') {
    url = Promise.resolve(__non_webpack_require__)('url')
  } else {
    try {
      url = import('url')
    } catch (err) {
      url = Promise.resolve(require('url'))
    }
  }

  url.then(url => { // rest of what's in the if (dartNodeIsActuallyNode)  check wrapped in this promise

Not sure why dynamic import is not used for both commonjs & ESM modules as it has been available in Node 14+ for both and dart-sass only supports Node 14+ anyway

@nex3
Copy link
Contributor

nex3 commented Jun 9, 2023

Not sure why dynamic import is not used for both commonjs & ESM modules as it has been available in Node 14+ for both and dart-sass only supports Node 14+ anyway

Dynamic import is asynchronous, and in order for Sass to remain consumable by CommonJS users it must be loadable synchronously.

@localnerve
Copy link

Was there a reason not to keep the default export for esm consumers? Seems bad to have an api divergence between to the two types of consumption.

@nex3
Copy link
Contributor

nex3 commented Jun 9, 2023

We never intended to expose a default export; that's not what's documented in our TypeScript type declarations, for example. I'm surprised to hear that that's the default way of consuming a CommonJS library that just sets exports.xyz from ESM in Node.js. A default export is generally not something libraries expose unless it's intended to be used directly as a function or type, which it is not in Sass.

That said, I think we can add a default export as well specifically to the Node.js library. I've filed #2008 to track that.

@ShenHongFei
Copy link

After upgrading the version, I got this error

import sass from 'sass';
       ^^^^
SyntaxError: The requested module 'sass' does not provide an export named 'default'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:122:21)

Then I changed the import method to

import * as sass from 'sass'

which fixed the problem

@kling90
Copy link

kling90 commented Jun 10, 2023

We had the same problem and after the update we had to change the import since it's no longer possible to import it as default in ESM.
In the documentation it says:

const sass = require('sass');

Which I don't think I'm alone to simply rewrite to:

import sass from 'sass';

Might be a good idea to add some examples for using the JS API with ESM here:
https://sass-lang.com/documentation/js-api/#md:usage

@noraj
Copy link

noraj commented Jun 10, 2023

We had the same problem and after the update we had to change the import since it's no longer possible to import it as default in ESM. In the documentation it says:

const sass = require('sass');

Which I don't think I'm alone to simply rewrite to:

import sass from 'sass';

Might be a good idea to add some examples for using the JS API with ESM here: https://sass-lang.com/documentation/js-api/#md:usage

Yeah is my gist moving from CJS to EJS for gulpfiles.

@localnerve
Copy link

@nex3
Sorry I think I've inadvertently misled here.

It's perhaps not just the default export that is the issue, rather the api is different for node applications depending on how the library is loaded (esm vs cjs).

This is my current 1.63.3 experience

CJS

const sass = require('sass')
// this works (reading sass.types)
const sassNull = sass.types.Null

ESM

import * as sass from 'sass'
// this doesn't work (reading sass.types)
const sassNull = sass.types.Null
  // TypeError: Cannot read properties of undefined (reading 'Null')

So maybe coincidentally this is a default export issue, but my experience is that the api has changed for node applications depending on how the library is loaded

This becomes a non-trivial divergence issue for downstream consumers that provide sass helper libraries that use dart-sass that support CJS and ESM loading.

@localnerve
Copy link

@nex3 Sorry I think I've inadvertently misled here.

It's perhaps not just the default export that is the issue, rather the api is different for node applications depending on how the library is loaded (esm vs cjs).

This is my current 1.63.3 experience

CJS

const sass = require('sass')
// this works (reading sass.types)
const sassNull = sass.types.Null

ESM

import * as sass from 'sass'
// this doesn't work (reading sass.types)
const sassNull = sass.types.Null
  // TypeError: Cannot read properties of undefined (reading 'Null')

So maybe coincidentally this is a default export issue, but my experience is that the api has changed for node applications depending on how the library is loaded

This becomes a non-trivial divergence issue for downstream consumers that provide sass helper libraries that use dart-sass that support CJS and ESM loading.

This appears to be fully addressed in #2010

christopherpickering added a commit to atlas-bi/Hub that referenced this issue Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants