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

Implement sass --embedded in pure JS mode #2413

Open
wants to merge 6 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 ntkme force-pushed the embedded-compiler branch from d3c7314 to 1cb26f3 Compare November 16, 2024 00:48
@ntkme
Copy link
Contributor Author

ntkme commented Nov 16, 2024

Ok, one more quirk for node worker. The way stdout/stderr stream in the worker thread works is that the worker posts message to the main thread and then the main thread writes the message asynchronously. So we cannot eagerly exit the main isolate, or otherwise the error message that the worker printed to stderr might get lost.

@ntkme ntkme force-pushed the embedded-compiler branch 2 times, most recently from 5d9c449 to 2fff2e7 Compare November 16, 2024 00:52
@ntkme ntkme force-pushed the embedded-compiler branch 3 times, most recently from 5233c41 to 7dc1e92 Compare December 5, 2024 19:03
@ntkme ntkme force-pushed the embedded-compiler branch 2 times, most recently from d0c8c1f to 5fa8f0d Compare December 10, 2024 18:08
@Goodwine Goodwine requested a review from nex3 December 10, 2024 21:05
@ntkme ntkme force-pushed the embedded-compiler branch 3 times, most recently from 1ebb5d1 to 2d1e04d Compare December 12, 2024 23:15
@ntkme ntkme force-pushed the embedded-compiler branch 2 times, most recently from f0ea835 to b19c6a8 Compare January 6, 2025 17:54
@ntkme ntkme force-pushed the embedded-compiler branch from b19c6a8 to 97d5c35 Compare January 14, 2025 16:35
@ntkme ntkme force-pushed the embedded-compiler branch 2 times, most recently from f6e6c4b to 1e8e188 Compare January 24, 2025 20:38
@ntkme ntkme force-pushed the embedded-compiler branch 3 times, most recently from 7d10b08 to 6ef6075 Compare February 6, 2025 00:07
@ntkme ntkme force-pushed the embedded-compiler branch 2 times, most recently from 77045ba to 37060c0 Compare February 14, 2025 18:08
@ntkme
Copy link
Contributor Author

ntkme commented Feb 15, 2025

@nex3 I can see that the team is focusing most of the time on the postcss sass parser and this pull request has low priority.

However, I would like to get this done in next few month before dart 3.8 become stable so that we can offer this as a replacement for dart ia32. Please take a look when you get a chance and let me know if you have any questions.

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
2 participants