sass / dart-sass

The reference implementation of Sass, written in Dart.
https://sass-lang.com/dart-sass
MIT License
3.97k stars 360 forks source link

Document 1.63.0 breaking change / 1.63.3 doesn't seem to fix it #2011

Closed noraj closed 1 year ago

noraj commented 1 year ago

While updating sass 1.62.1 → 1.63.2 I encounter the following error:

import dartSass from 'sass';
       ^^^^^^^^
SyntaxError: The requested module 'sass' does not provide an export named 'default'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:438:15)

It seems that 1.63.0 is including a breaking change I don't see listed in the changelog or the release note.

Note: It's in an ESM file: https://gitlab.com/rawsec/rawsec-cybersecurity-list/-/blob/master/gulpfile.mjs#L8

I saw this message https://github.com/sass/dart-sass/issues/1995#issuecomment-1581784720 in #1995 but 1.63.1 and 1.63.2 don't seem to fix it.

1.66.3 (https://github.com/sass/dart-sass/pull/2006/files) claims to fix it.

1) However, I suggest updating 1.63.0 changelog to notify the breaking change bug and tell 1.63.3 release fix it so this bug is documented. Just having 1.63.3 release note saying it fixed a bug is not enough, because automatic dependency update bot will fetch release not, and when updating to 1.63.0 nothing in it says there is a breaking change.

2) I just tested 1.63.3 and I still have:

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

As a workaround I had to change:

-import dartSass from 'sass';
+import * as dartSass from 'sass';

So this is even more a breaking change that should be documented in the changelog as well as the import method syntax upgrade needed.

developerr-ayush commented 1 year ago

thanks @noraj its working

noraj commented 1 year ago

2010 may fix the import but not the doc

brunowacemberg commented 1 year ago

Thanks, @noraj !

tobiu commented 1 year ago

Hi guys,

the v1.63.x changes also broke the https://github.com/neomjs/neo theme-builder and -watcher. The required change is very similar to what @noraj did:

As a workaround I had to change:

-import sass from 'sass';
+import * as sass from 'sass';

=> https://github.com/neomjs/neo/commit/4460164e051edb982f49dfea6f5d14cda26e8748

Please document breaking changes inside the changelog or use new major versions or beta tags inside the versioning to highlight this. Thx!

Goodwine commented 1 year ago

Breaking changes are when we intentionally make a change that is not compatible with earlier versions. In this particular case, the release did break things but it was unintentional. In other words, a bug. The bug was fixed in 1.63.3 and the CHANGELOG.md file was updated calling it out.

So instead, I think instead you are proposing that whenever there is a noteworthy bug in a release then the CHANGELOG.md file and release notes should be amended, right?

Goodwine commented 1 year ago

Assigning to @nex3 for additional input.

ghiscoding commented 1 year ago

my personal take on this issue is that the v1.63.3 does not fix it for me either, I guess that I will have to do the proposed workaround (not ideal though)

-import sass from 'sass';
+import * as sass from 'sass';

because default export are no longer provided.... <-- well to me this is a breaking change since I have to change my code.

It is also the second time that this project releases breaking change under a minor (which is supposed to be reserved for new features only). The previous time v1.33.0 was a much bigger deal since it flooded users with all the math warnings in our console... I was hoping that it would never happen again but it seems to have happen again with 1.63.0 (exactly 30 bumps since then, perhaps a coincidence lol)...

It would be helpful and avoid less problems if this project could follow a stricter semver though I understand that it might sometime be hard to detect a breaking change. It ok to release major breaking change every now and then.

this is my personal take on the subject, it's ok to disagree with me. Cheers and thanks for the great lib

nex3 commented 1 year ago

Duplicate of #2008

noraj commented 1 year ago

Hi @nex3, I was feeling it's not a duplicate because this issue focus more on documenting the breaking change / reporting the lack of documentation about it, that reporting the issue itself #1995 or asking to re-introduce backward compatibility #2008.

nex3 commented 1 year ago

Since the breaking change wasn't intentional, the appropriate fix is to un-break it rather than document the breakage.

noraj commented 1 year ago

In think both are needed. Else people can still unexpectedly update to a broken version.

nex3 commented 1 year ago

There are always going to be intermediate versions that are broken in one way or another. That's the nature of introducing and fixing bugs.