fix(sass): replace default import with ESM module import

[roman-petrov]

Description

Replaces default import from sass with module import. Default sass import is deprecated and does not work with tsx imports.

Without this change we get Pre-transform error: [sass] Cannot read properties of undefined (reading 'initAsyncCompiler') error and it is not possible for us to migrate to sass compiler API + sass-embedded package.

@hi-ogawa, can you please help reviewing this?

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[hi-ogawa]

Hmm, interesting. Actually I don't quite remember why I had .default there. Now looking at sass-embedded output, it looks like export default { ... } is only shimmed manually in addition to normal named exports, so .default on our side doesn't look necessary https://github.com/sass/embedded-host-node/blob/b54575994a4c51a0e6cfa3fbed236923609245b6/lib/index.mjs#L54.

I can look into this further. Btw, can you provide a reproduction to show how this loading fails with tsx? Technically, there might be a different resolution issue on tsx side.

[roman-petrov]

Thanks for a fast response. Actually, I don't fully understand the source of this issue and I agree it might be better to have a fix for this in tsx. While researching this issue I noticed that sass-embedded ships with two exports CommonJS and ESM (index.js and index.mjs). index.mjs has default export, but index.js does not.

Hmm, interesting. Actually I don't quite remember why I had .default there.

I also noticed that without .default some test fail under Node 22 (strange, why?..), that's why I added the workaround as a fallback in the PR.

So it's my laziness, sorry for that :) I will try to prepare a reproduction for this issue with vite + tsx, but I am still not sure if I will be able to make a PR in tsx myself. Maybe I will have to ask tsx contributors for a help.

[roman-petrov]

Just tried to create a minimal reproduction from scratch with Vite + tsx but no success:(

• Created a simple Vite + React project with scss
• Added sass-embedded package
• Addded vite.config.ts with scss: { api: 'modern-compiler' }

When I start Vite on this project using set NODE_OPTIONS='--import tsx' && vite everything works fine and I don't see the issue.

So maybe this issue is related with our company (quite large) project Vite plugins or other environment... (maybe, it is not related to tsx at all?...) This quest might be quite hard and time consuming so at the moment I am not ready for it...:)

@hi-ogawa WDYT, can this PR be merged or it is better just to close it and wait for future solutions?...

[hi-ogawa]

I think I remembered why it uses .default. sassPath in import(pathToFileURL(sassPath).href)) is obtained via require.resolve:

vite/packages/vite/src/node/utils.ts

Line 1057 in 321b213

return _require.resolve(id, { paths: [root, _dirname] })

and this picks up sass-embedded's cjs entry dist/lib/index.js instead of dist/lib/index.mjs. When import-ing cjs file on NodeJs, it's safer to rely on default export, which corresponds to cjs module.exports global. So Vite using .default here is sound (though probably Vite should move to resolving package's esm entry at some time point).

So maybe this issue is related with our company (quite large) project Vite plugins or other environment... (maybe, it is not related to tsx at all?...) This quest might be quite hard and time consuming so at the moment I am not ready for it...:)
@hi-ogawa WDYT, can this PR be merged or it is better just to close it and wait for future solutions?...

Unless there's a reproduction to understand the issue, it's unlikely we can take an action here unfortunately. We'll probably close this PR unless anyone has some ideas what's going on.

[roman-petrov]

@hi-ogawa Thank you for the help! I am closing this for now.