Skip to content

Implement sass --embedded in pure JS mode #2413

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

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

ntkme
Copy link
Contributor

@ntkme ntkme commented Oct 26, 2024

Closes #2325.

sass/embedded-host-node#344

Implementation

The actual isolate dispatcher and compilation dispatcher are nearly unchanged. However, I had to replace isolate with worker communication, and mock tons of small things that do not work on node.

Testing

  • All Dart embedded tests are passing. - GitHub CI has been updated to run these in this PR.
  • All JS API tests are passing. - GitHub CI has been updated to run these in this PR.
  • All Ruby API tests are passing.

@ntkme ntkme force-pushed the embedded-compiler branch from 8d7d4de to 7084da7 Compare October 26, 2024 02:31
@ntkme ntkme marked this pull request as ready for review October 26, 2024 02:50
@ntkme ntkme marked this pull request as draft October 26, 2024 03:40
@ntkme ntkme force-pushed the embedded-compiler branch 2 times, most recently from e66df5b to d53fcc5 Compare October 26, 2024 17:27
@ntkme ntkme force-pushed the embedded-compiler branch 9 times, most recently from d7e6206 to b3794eb Compare October 28, 2024 06:35
@ntkme ntkme marked this pull request as ready for review October 28, 2024 06:49
@ntkme ntkme force-pushed the embedded-compiler branch 3 times, most recently from 9112b44 to 54fabf3 Compare October 28, 2024 07:42
@ntkme ntkme force-pushed the embedded-compiler branch 8 times, most recently from 257b4fd to ac718ee Compare October 28, 2024 21:03
@ntkme
Copy link
Contributor Author

ntkme commented Apr 8, 2025

Applied most of the feedbacks. However, I have a awkward feeling about mixing classic js interop and new js interop in the same file, which is why I have lots of js specific things with new interop are only in embedded folder instead of in the generic folder.

@nex3 I haven't changed these yet, as I want to hear what's your thought on this.

@ntkme
Copy link
Contributor Author

ntkme commented Apr 8, 2025

Another note: The reason I tried to avoid classic interop and node_introp is that I couldn't get channel.port2 passed to Worker() constructor correctly with node_interop. Neither allowInterop() nor explicit casting worked, and it was a nightmare trying to debug what went wrong. I looked at node_interop repo trying to see if there is any example for it and there is no actual usage of transferList anywhere.

The new dart 3.3 js interop on the other hands was way more straight forward to use and debug. I got Worker() working on my first attempt.

@ntkme ntkme force-pushed the embedded-compiler branch from 48d68dc to 89175db Compare May 8, 2025 19:58
@Goodwine Goodwine requested a review from nex3 May 13, 2025 20:08
@ntkme ntkme force-pushed the embedded-compiler branch from 89175db to 672239a Compare May 13, 2025 22:16
@ntkme ntkme force-pushed the embedded-compiler branch from 672239a to 5231955 Compare May 20, 2025 19:56
@ntkme ntkme force-pushed the embedded-compiler branch from 5231955 to 5aba167 Compare May 21, 2025 20:42
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.

Implement sass --embedded in pure JS mode
3 participants