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

JS API tests for loading multiple nested paths with renderSync #1910

Merged
merged 5 commits into from
Jul 1, 2023

Conversation

Goodwine
Copy link
Member

@Goodwine Goodwine commented May 17, 2023

Issue: sass/dart-sass#1962

[skip sass-embedded]

See: sass/embedded-host-node#226

js-api-spec/importer.test.ts Outdated Show resolved Hide resolved
@Goodwine Goodwine force-pushed the nested-multiple-imports branch 2 times, most recently from d92ed37 to 477dd43 Compare May 17, 2023 23:54
js-api-spec/legacy/importer.test.ts Outdated Show resolved Hide resolved
js-api-spec/legacy/importer.test.ts Outdated Show resolved Hide resolved
js-api-spec/legacy/importer.test.ts Outdated Show resolved Hide resolved
@nex3
Copy link
Contributor

nex3 commented May 18, 2023

What parameters are actually passed to the importer in plain Dart Sass versus the embedded host?

@Goodwine
Copy link
Member Author

What parameters are actually passed to the importer in plain Dart Sass versus the embedded host?

I'm not 100% sure if this is what you're referring to, but:

dart-sass importer

console.log
  ## DEBUG: url= ./foo/_a.scss - prev: /home/goodwine/repos/sass-spec/spec/sandbox/9067981249289676/src/index.scss
console.log
  ## DEBUG: url= ./foo/_a.scss - prev: /home/goodwine/repos/sass-spec/spec/sandbox/9067981249289676/src/_includes.scss
console.log
  ## DEBUG: url= ./foo/_b.scss - prev: /home/goodwine/repos/sass-spec/spec/sandbox/9067981249289676/src/_includes.scss

embedded importer

console.log
  ## DEBUG: url= foo/_a.scss - prev: /home/goodwine/repos/sass-spec/spec/sandbox/308212682427188/src/index.scss
console.log
  ## DEBUG: url= file:///home/goodwine/repos/sass-spec/spec/sandbox/308212682427188/src/foo/_b.scss - prev: /home/goodwine/repos/sass-spec/spec/sandbox/308212682427188/src/_includes.scss
<crash>

I looked at the implementations, and dart-sass seems to just call the importer without trying to do much wrapping, but embedded-host-node has some fancier wrapping and I think that's where it goes "wrong"

Like if I implement a "modern" importer using compile() instead of renderSync() I would have to implement a correct canonalize() and I would realize that the input Sass is kinda wrong 🤔

js-api-spec/legacy/importer.node.test.ts Outdated Show resolved Hide resolved
@Goodwine Goodwine merged commit b926940 into main Jul 1, 2023
@Goodwine Goodwine deleted the nested-multiple-imports branch July 1, 2023 02:17
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

Successfully merging this pull request may close these issues.

3 participants