-
Notifications
You must be signed in to change notification settings - Fork 217
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
Convert Project to TypeScript #226
base: v5
Are you sure you want to change the base?
Conversation
68e83a8
to
fb69b46
Compare
fb69b46
to
fde1255
Compare
this is wanted with a new option "mergeWithTarget"
fde1255
to
254d50c
Compare
export function getFullOptions<O extends Options>(inputOptions?: O) { | ||
const overrides = inputOptions === undefined | ||
? undefined | ||
: Object.fromEntries( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TehShrike
Object.fromEntries
requires node 12+. If we want to support node 10 (lts ends 2021-04-30) we'll have to either polyfill this or do it in another way (which shouldn't be too big a deal).
@RebeccaStevens Hey I think you did a great job, what about creating a separate package with implementation from the current pr? I guess it brings much more value than wait until the maintainer decides to accept your work. I'll be your first user :dd |
We want to use your implementation in Osome.com, and I faced the next dilemma to copy-paste and go or use your package and maintain it together. Wait for your thoughts. |
I'd be happy to. I've actually been thinking about doing this already but haven't got round to it. I've also got some ideas on improving the functionality. I'd probably also end up dropping native support for es5. |
@RebeccaStevens I guess the name is doesn't matter in terms of the end user. But the one way to think about it is how to get more SEO traffic :d So it can be ts-merge-deep So anyway, wait for your next steps 💯 |
@frolovdev I've just release my own take on deep merge. https://www.npmjs.com/package/deepmerge-ts
|
No description provided.