Skip to content

Conversation

@MartyJRE
Copy link
Contributor

@MartyJRE MartyJRE commented Jul 13, 2025

This commit sets includeSymbols to false, resulting in not throwing when calling RegExp.exec on them.

ljharb/js-traverse#5
ljharb/js-traverse@aab373f

Fixes #180

Checklist

  • DCO (Developer Certificate of Origin) signed in all commits
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Commit messages are following our guidelines

@MartyJRE MartyJRE requested a review from raymondfeng as a code owner July 13, 2025 12:44
@MartyJRE MartyJRE force-pushed the fix-180-traverse-symbols branch 2 times, most recently from 92cd827 to 2f3b705 Compare July 14, 2025 11:01
@MartyJRE
Copy link
Contributor Author

Hi @raymondfeng, fixed the commit message lint. Thanks!

@dhmlau
Copy link
Member

dhmlau commented Jul 14, 2025

@MartyJRE, I believe the commit lint issue was not fixed yet. If you squash the commits, it should be ok.
Your changes LGTM. Let's wait for @raymondfeng's review too.

@MartyJRE MartyJRE force-pushed the fix-180-traverse-symbols branch from 2f3b705 to 482cf07 Compare July 14, 2025 16:04
@MartyJRE
Copy link
Contributor Author

@MartyJRE, I believe the commit lint issue was not fixed yet. If you squash the commits, it should be ok.

Your changes LGTM. Let's wait for @raymondfeng's review too.

You're absolutely right. I blanked about the second commit. Thanks!

@MartyJRE
Copy link
Contributor Author

Should I also add a new test case for symbols specifically?

@dhmlau
Copy link
Member

dhmlau commented Jul 14, 2025

Should I also add a new test case for symbols specifically?
@MartyJRE, that's a good idea.

@MartyJRE MartyJRE force-pushed the fix-180-traverse-symbols branch from 482cf07 to 9bb3e2b Compare July 14, 2025 17:16
@MartyJRE
Copy link
Contributor Author

@dhmlau Added tests. Sorry for the reformat, but I felt it might be good to add a test for compile as well, so I split them up into two describes.

@MartyJRE MartyJRE force-pushed the fix-180-traverse-symbols branch from 9bb3e2b to 416a390 Compare July 14, 2025 17:29
@MartyJRE MartyJRE requested a review from samarpanB July 14, 2025 17:29
@MartyJRE MartyJRE force-pushed the fix-180-traverse-symbols branch from 416a390 to b96fd0d Compare July 14, 2025 17:35
@MartyJRE MartyJRE changed the title fix: do not include symbols in js-travers fix: do not include symbols in js-traverse Jul 14, 2025
@MartyJRE MartyJRE force-pushed the fix-180-traverse-symbols branch from b96fd0d to aec7989 Compare July 14, 2025 17:48
This commit sets `includeSymbols` to false, resulting in not throwing
when calling RegExp.exec on them.

Closes loopbackio#180

Signed-off-by: Martin Pražák <martin.prazak@lutherone.com>
@MartyJRE MartyJRE force-pushed the fix-180-traverse-symbols branch from aec7989 to f35cae4 Compare July 14, 2025 17:55
@dhmlau dhmlau merged commit f607468 into loopbackio:master Jul 15, 2025
5 checks passed
@dhmlau
Copy link
Member

dhmlau commented Jul 15, 2025

@MartyJRE, thanks for your contribution. Your PR has landed! 🎉

@MartyJRE MartyJRE mentioned this pull request Jul 29, 2025
5 tasks
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.

traverse dependency breaking change

3 participants