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

Replace traverse with neotraverse #806

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Replace traverse with neotraverse #806

merged 1 commit into from
Jul 22, 2024

Conversation

wojtekmaj
Copy link
Contributor

This PR replaces traverse with neotraverse, a fork and TypeScript rewrite of traverse with 0 dependencies (as opposed to 66: https://npmgraph.js.org/?q=traverse) and lots of improvements.

Copy link

@PuruVJ PuruVJ left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR! Left some comments

lib/bundle.js Outdated Show resolved Hide resolved
lib/deref.js Outdated Show resolved Hide resolved
lib/relatedFiles.js Outdated Show resolved Hide resolved
lib/schemaUtils.js Outdated Show resolved Hide resolved
@thim81
Copy link
Contributor

thim81 commented Jul 20, 2024

@VShingala I can recommend switching from the old traverse to neotraverse. It is fully backward compatible and for my NPM project it reduced the children dependencies from 66 to 0.

package.json Outdated Show resolved Hide resolved
This PR replaces traverse with neotraverse, a fork and TypeScript rewrite of traverse with 0 dependencies (as opposed to 66: https://npmgraph.js.org/?q=traverse) and lots of improvements.
@VShingala
Copy link
Member

Thanks for the contribution! I've checked out the neotraverse and related codebase and it looks good. It's also not breaking any of our existing use-cases so I'm going ahead with merging this.

@VShingala VShingala merged commit 7b67a52 into postmanlabs:develop Jul 22, 2024
6 checks passed
@VShingala
Copy link
Member

@wojtekmaj @PuruVJ I had not checked this till I released the new version of this module but it seems neotraverse is not compatible with older node versions. i.e. Using it with node v16 gave below error. Since this module still supports older versions of modules. I'll revert this change for now and keep the PR open. Once we do come with major version and move out from older version, we can start using this module. But till then, using this module will be breaking change for existing consumers.

error [email protected]: The engine "node" is incompatible with this module. Expected version ">= 18". Got "16.10.0"

@PuruVJ
Copy link

PuruVJ commented Jul 22, 2024

Good observation! I just released v0.6.15 with node > 10. Should work now.

@wojtekmaj wojtekmaj deleted the neotraverse branch July 23, 2024 08:39
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.

4 participants