Skip to content

feat: omitServerProps helper function#277

Open
gmaclennan wants to merge 1 commit intomainfrom
feat/omit-server-props
Open

feat: omitServerProps helper function#277
gmaclennan wants to merge 1 commit intomainfrom
feat/omit-server-props

Conversation

@gmaclennan
Copy link
Copy Markdown
Member

I needed a more flexible version of valueOf for removing server-managed props from a Mapeo Doc. In the front-end we are modifying some props such as replacing refs with actual docs, but we still need to be able to strip server properties when editing.

I decided to create a separate function since this is subtly different from valueOf, and the name valueOf isn't great anyway.

@gmaclennan gmaclennan self-assigned this Dec 3, 2024
@gmaclennan gmaclennan requested a review from EvanHahn December 3, 2024 21:52
Copy link
Copy Markdown
Contributor

@EvanHahn EvanHahn left a comment

Choose a reason for hiding this comment

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

  1. I was initially surprised to see that the word "server" meant "managed by the backend"...maybe there's a better name for this?
  2. I admit I don't fully understand the motivation for this change but I trust that y'all have discussed it and have a good reason.
  3. Do we have a plan to deprecate valueOf?

Comment thread src/lib/utils.ts
export function omitServerProps<TDoc extends MapeoCommon>(
doc: TDoc & { forks?: string[] }
) {
return excludeKeys(doc, keysToExclude)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use our existing omit helper (and maybe remove the new filter-obj dependency)?

Comment thread src/lib/utils.ts
}

/**
* Get the value of a document, excluding server-managed properties.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this comment is a little unclear because it makes it look like it should do the same thing as the other function.

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