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.

Sign up for GitHub

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

Jump to bottom

support deno #2745

Closed
wants to merge 17 commits into from
Closed

support deno #2745

wants to merge 17 commits into from

Conversation

vixalien
Copy link
Contributor

@vixalien vixalien commented Dec 27, 2022 •
edited

What:

This PR allows stylus to work on deno, an alternative to Node.JS

Why:

Deno is gaining traction, and I personally use stylus, so I wanted to contrinue using stylus with Deno (after I switched to it from Node)

How:

The fix is two-steps:

  1. Use Object.setPrototypeOf instead of the deprecated __proto__ (which Deno doesn't support)
  2. Fix a call in the boolean node that called the Boolean function, causing deno to run into an infinite loop as it was calling the constructor from the constructor in a recursive way.

I THINK THIS IS OKAY, BUT PLEASE VERIFY

Checklist:

  • Documentation N/A
  • Unit Tests N/A
  • Code complete
  • Changelog

May I recommend moving to ES6 classes (would help) in the future, instead of modifying the prototype chain manually as this results in code reliability & performance issues.

@netlify
Copy link

netlify bot commented Dec 27, 2022 •
edited

✅ Deploy Preview for stylus-docs canceled.

Name Link
🔨 Latest commit 2b227d7
🔍 Latest deploy log https://app.netlify.com/sites/stylus-docs/deploys/63aa64f5009b3400082b43a6

@vixalien vixalien changed the title fix deno running infinite loop support deno Dec 27, 2022
@vixalien
Copy link
Contributor Author

I forgot to add that this is a very minor change (in face of the user) as the API is unmodified and everything (including tests) works exactly as expected.

@vixalien
Copy link
Contributor Author

vixalien commented Apr 5, 2023

anyone?

@iChenLei
Copy link
Member

iChenLei commented Apr 6, 2023

Can you add github action ci for stylus with deno environment ?

@netlify
Copy link

netlify bot commented Aug 11, 2023 •
edited

✅ Deploy Preview for stylus-docs ready!

Name Link
🔨 Latest commit ce7adc9
🔍 Latest deploy log https://app.netlify.com/sites/stylus-docs/deploys/64d8b843cba11d000867f09c
😎 Deploy Preview https://deploy-preview-2745--stylus-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@vixalien
Copy link
Contributor Author

@iChenLei ptal

iChenLei
iChenLei requested changes Aug 11, 2023
View reviewed changes
lib/nodes/object.js Outdated Show resolved Hide resolved
@vixalien
Copy link
Contributor Author

@iChenLei fixed

@vixalien
fix this.Objectpointing to custom Object
42eafda 
this solution works for:

- newer node versions (12+) and deno where `globalThis` is defined
- old node versions (where `global` is defined)
@vixalien vixalien requested a review from iChenLei August 12, 2023 23:42
dependabot bot and others added 9 commits August 13, 2023 12:34
@dependabot @vixalien
chore(deps): bump undici from 5.10.0 to 5.19.1 in /docs (#2758)
aadf267 
Bumps [undici](https://github.com/nodejs/undici) from 5.10.0 to 5.19.1.
- [Release notes](https://github.com/nodejs/undici/releases)
- [Commits](nodejs/undici@v5.10.0...v5.19.1)

---
updated-dependencies:
- dependency-name: undici
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@iChenLei @vixalien
ci: update nodejs matrix test suites (#2765)
9430b49 
* ci: update nodejs matrix test suite

* Update ci.yml
@peterhauke @vixalien
Correct typos and copy edit (#2764)
da81649 
Fix capitalization of Node.js, CSS, Stylus (CSS is already plural so CSSs is not really correct)

Correct minor language errors
@hooozen @vixalien
doc(js.md): supplement parameters description (#2780) (#2781)
17350fe
@taraspolovyi @vixalien
add support for CSS Values 4 viewport-relative units (#2775)
d05fc0d 
Co-authored-by: Taras Polovyi <taras.polovyi@grammarly.com>
@iChenLei @vixalien
chore: update pull request template (#2784)
ff18aea
@dependabot @vixalien
chore(deps): bump json5 from 2.2.1 to 2.2.3 in /docs (#2747)
e6fe347 
Bumps [json5](https://github.com/json5/json5) from 2.2.1 to 2.2.3.
- [Release notes](https://github.com/json5/json5/releases)
- [Changelog](https://github.com/json5/json5/blob/main/CHANGELOG.md)
- [Commits](json5/json5@v2.2.1...v2.2.3)

---
updated-dependencies:
- dependency-name: json5
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@iChenLei @vixalien
ci: add nodejs v20 test (#2791)
b5c08f3
@dependabot @vixalien
chore(deps): bump vite from 2.9.15 to 2.9.16 in /docs (#2799)
0d0cedc 
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 2.9.15 to 2.9.16.
- [Release notes](https://github.com/vitejs/vite/releases)
- [Changelog](https://github.com/vitejs/vite/blob/v2.9.16/packages/vite/CHANGELOG.md)
- [Commits](https://github.com/vitejs/vite/commits/v2.9.16/packages/vite)

---
updated-dependencies:
- dependency-name: vite
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@vixalien
Copy link
Contributor Author

@iChenLei I believe stylus's dependencies do not support older versions of node anymore.

In the logs you can see mocha says

unsupported engine for mocha@9.2.2: wanted: {"node":">= 12.0.0"} (current: {"node":"10.24.1","npm":"6.14.12"})

also @adobe/css-tools causes the test to fail since it uses an optional chaining operator (?.) which works on node v14+.

The mocha warning can be ignored. but the @adobe/css-tools one cannot be ignored. we'll need to use an older version of this package if supporting older versions of node is a must.

@vixalien
Copy link
Contributor Author

@iChenLei ptal

iChenLei
iChenLei reviewed Aug 14, 2023
View reviewed changes
Copy link
Member

@iChenLei iChenLei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question: why not directly convert function into class (only needing Node.js v4+), instead of using setPrototypeOf? Plus, you don't need to care about the issues with @adobe/css-tools.

@vixalien
Copy link
Contributor Author

that can be done yes, I was surprised why stylus does this too. since it'll be a huge change, I'll be working on a separate PR. stay tuned

@vixalien vixalien mentioned this pull request Aug 14, 2023
Merged
4 tasks
@vixalien
Copy link
Contributor Author

@iChenLei made a PR for Classes in #2813

@iChenLei
Copy link
Member

iChenLei commented Aug 14, 2023 •
edited

So, can we close this PR ?

@vixalien
Copy link
Contributor Author

yes.

@vixalien vixalien closed this Aug 14, 2023
@vixalien vixalien deleted the deno branch August 14, 2023 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iChenLei iChenLei iChenLei requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@vixalien @iChenLei @peterhauke @hooozen @taraspolovyi