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

PR: Fix Autopath and Path for primitive types #253

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vonBrax
Copy link

@vonBrax vonBrax commented Sep 24, 2021

🎁 Pull Request

  • Used a clear / meaningful title for this pull request
  • Tested the changes in your own code (on your projects)
  • Added / Edited tests to reflect changes (tests folder)
  • Have read the Contributing part of the Readme
  • Passed npm test

Fixes

#246
#228

Why have you made changes?

To fix the above mentioned issues. Autopath currently does not work for primitive types since they don't have a "next path".

What changes have you made?

  • Added a new type CurrentPath in Autopath to retrieve the value of the current path if there's no next path, using the inverse logic for NextPath
  • Added a filter in Path to exclude methods from primitive types, since Path currently return their types (ex: a.someStr.toUpperCase returns () => string). This way Path and Autopath have a more consistent behavior when working together.

What tests have you updated?

  • tested primitive types, array indexes and functions in tests/Function.ts
  • tested paths to primitive methods in tests/Object.ts

Is there any breaking changes?

  • Yes, I changed the public API & documented it
  • Yes, I changed existing tests
  • No, I added to the public API & documented it
  • No, I added to the existing tests
  • I don't know

@gautelo
Copy link

gautelo commented Sep 6, 2022

Any chance you can have a look at this @millsp ? AutoPath is a pretty cool feature, so would be nice if it could get fixed. 🙏

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.

2 participants