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

Fluent API (second attempt) #161

Closed
wants to merge 1 commit into from
Closed

Conversation

stefaanv
Copy link
Contributor

@stefaanv stefaanv commented Aug 9, 2024

Summary

As discussed in PR#131, a first example of fluent interface for mapValues and unique functions.
Haven't beem able to exclude usage to when radashi/fluent is imported though. Lacking the knowledge for that, unfortunately. Maybe someone can help there

Related issue, if any:

None

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed
  • Related benchmarks have been added or updated, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size

@stefaanv
Copy link
Contributor Author

stefaanv commented Aug 9, 2024

@MarlonPassos-git, my second attempt to create a PR for the fluent API after your comment on PR#159.
The intent is - of course - to extend the fluent API to other functions if this PR gets accepted.
I have set PR#159 to draft, it can be closed.
Please let me know if I need to do something else, I am not experienced with the workflow radashi is using.

@stefaanv stefaanv marked this pull request as draft August 9, 2024 14:00
@stefaanv
Copy link
Contributor Author

stefaanv commented Aug 9, 2024

Set to draft because I noticed that other tests don't pass anymore.

@stefaanv stefaanv closed this Aug 9, 2024
@MarlonPassos-git
Copy link
Contributor

Anticipating some points we aim to address in this PR, in addition to the implementation of the Fluent API, which we can discuss further:

  • Detailed documentation for each new or modified method added to the API, located in the docs folder.
  • Creation of a more generic documentation page that provides an overview of the Fluent API.
  • Initial setup for the separation of the fluent module, considering the idea is to create a separate package as previously discussed.
    • Adjustments to the build process so that Radashi can be compiled without including the Fluent modules.
    • Preparation of the build process for the radashi-fluent package.

@MarlonPassos-git
Copy link
Contributor

If the idea is to keep two projects in the repository I would think about adjusting the package to use workspaces using something like turboRepo

or just really think about moving the fluent logic to another repository.
what do you think? @aleclarson

@aleclarson aleclarson reopened this Aug 10, 2024
@aleclarson
Copy link
Member

I think the only way this is gonna work is if we generate large parts of the fluent module. Otherwise, there's not enough incentive to warrant the additional maintenance burden (which appears to be quite a lot). Of course, code generation is not everyone's forté (I could certainly write it, but I can't justify the time expense). It would likely involve use of ts-morph, since we would need to remove the first argument from the type signatures.

Apologies for not having this realization sooner. Hopefully, not too much time was lost on your end. That said, feel free to develop this PR further, as others may find it valuable as Radashi grows. Alternatively, you could develop a radashi-fluent package and publish it yourself.

@stefaanv
Copy link
Contributor Author

I can do most of the functions in radashi, not all of them because things like get will interfere badly with other popular packages.
Can the radashi-fluent repo live in the radashi-org github space ?
Also, I could use some help in setting up the dual cjs/esm workflow.

@aleclarson
Copy link
Member

aleclarson commented Aug 12, 2024

I can do most of the functions in radashi, not all of them because things like get will interfere badly with other popular packages.

For the src/object/ functions, it'd be better to add them to the Object namespace like the JS stdlib does with Object.keys(). Otherwise, you risk collisions with custom object properties.

Also, I could use some help in setting up the dual cjs/esm workflow.

That's relatively straight-forward. Install tsup and copy our tsup.config.ts file. Then run pnpm tsup or add "build": "tsup" to package.json scripts so you can do pnpm build instead.

Can the radashi-fluent repo live in the radashi-org github space ?

Sure, I've invited you to the radashi-fluent repo here: https://github.com/radashi-org/radashi-fluent

If you don't see the invite there, check your email.

Once you've accepted the invite, you'll be able to push to that repository, manage its issues, merge pull requests to it, etc. But only for that repository. I'll let you handle the publishing to NPM. Basically, you're the leader of that repository.

@aleclarson
Copy link
Member

aleclarson commented Aug 14, 2024

@stefaanv I've made the radashi-fluent repository private until you're ready to publish its first version. You can change its visibility on the settings page. See here for detailed guidance, if you need it.

Also, I've invited you on NPM to be a maintainer of the radashi-fluent package, so you can publish new versions. I've enabled two-factor authentication as a requirement, for security reasons. Here's a guide on setting that up on your end.

@aleclarson aleclarson closed this Aug 14, 2024
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