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

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

Closed
noraj opened this issue Jun 10, 2023 · 12 comments
Closed

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

noraj opened this issue Jun 10, 2023 · 12 comments
Assignees

Comments

@noraj
Copy link

noraj commented Jun 10, 2023

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 #1995 (comment) 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
Copy link

thanks @noraj its working

@noraj
Copy link
Author

noraj commented Jun 11, 2023

#2010 may fix the import but not the doc

@brunowacemberg
Copy link

Thanks, @noraj !

@tobiu
Copy link

tobiu commented Jun 12, 2023

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';

=> neomjs/neo@4460164

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

@Goodwine
Copy link
Member

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
Copy link
Member

Assigning to @nex3 for additional input.

@ghiscoding
Copy link

ghiscoding commented Jun 13, 2023

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
Copy link
Contributor

nex3 commented Jun 13, 2023

Duplicate of #2008

@nex3 nex3 marked this as a duplicate of #2008 Jun 13, 2023
@nex3 nex3 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2023
@noraj
Copy link
Author

noraj commented Jun 14, 2023

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
Copy link
Contributor

nex3 commented Jun 14, 2023

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

@noraj
Copy link
Author

noraj commented Jun 15, 2023

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

@nex3
Copy link
Contributor

nex3 commented Jun 15, 2023

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.

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

7 participants