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

avoid infinite loop in add-property #2772

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

dlants
Copy link

@dlants dlants commented Mar 7, 2023

What / Why:

#2771

How:

I suspect the infinite loop may be happening due to the local definition for add-property overshadowing the js definition, thereby creating an infinite loop. There seems to be a race condition for when that overshadowing happens, which occasionally allows the tests to pass.

Checklist:

  • Documentation
  • Unit Tests
  • Code complete
  • Changelog

@netlify
Copy link

netlify bot commented Mar 7, 2023

Deploy Preview for stylus-docs canceled.

Name Link
🔨 Latest commit 203ad7c
🔍 Latest deploy log https://app.netlify.com/sites/stylus-docs/deploys/64067f62e2a8b10007462290

@iChenLei
Copy link
Member

iChenLei commented Mar 7, 2023

Hey, can you add unit test for this PR ? Thanks

@dlants
Copy link
Author

dlants commented Mar 7, 2023

Hey, can you add unit test for this PR ? Thanks

Hm. I'm not sure if I can. It seems to me that there's a race condition, which causes this error in certain situations (as specified in #2771 ).

From my testing with my project's build tooling, the fix in this PR does seem to resolve the issue.

However, I'm not sure how to trigger it in a unit test. It seems like there are already unit tests for add-property ( test/cases/bifs.add-property.styl ), and those didn't catch this issue. Any suggestions?

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.

None yet

2 participants