Skip to content

Conversation

@CarlSchwan
Copy link

With snowflake-ids, we get numbers represented as strings as ids.

Not tested, as I couldn't figure out how to use a patched version in server but this should fix the CI failure on nextcloud/server#56183

With snowflake-ids, we get numbers represented as strings as ids.

Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
@CarlSchwan CarlSchwan requested a review from skjnldsv November 24, 2025 15:29
@CarlSchwan CarlSchwan self-assigned this Nov 24, 2025
@CarlSchwan CarlSchwan added 3. to review 3️⃣ Waiting for reviews type: enhancement 🚀 New feature or request labels Nov 24, 2025
@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.18%. Comparing base (550e782) to head (668f17a).
⚠️ Report is 34 commits behind head on main.

Files with missing lines Patch % Lines
lib/node/nodeData.ts 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1377      +/-   ##
==========================================
- Coverage   90.49%   90.18%   -0.31%     
==========================================
  Files          23       23              
  Lines         652      652              
  Branches      176      176              
==========================================
- Hits          590      588       -2     
- Misses         53       54       +1     
- Partials        9       10       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@artonge artonge requested a review from susnux November 25, 2025 10:11
@artonge
Copy link
Contributor

artonge commented Nov 25, 2025

I fear that supporting two types will not help developers to get their typing right. Not sure what's the best solution.

Going string all the way in a new major version might be the best.
Or having two distinct fields?
Or using a template <T>?

@susnux
Copy link
Contributor

susnux commented Nov 25, 2025

I think this will break quite some typings in apps like files - but maybe also other apps using it.

A new major version does not solve this unless we explicitly break compatibility with Nextcloud 33 saying that only files v4 will be compatible with Nextcloud 33+.
Then we can do that, but also have to adjust many places in the files app that expect a number to be returned by the Node interface.

@susnux
Copy link
Contributor

susnux commented Nov 29, 2025

@CarlSchwan

Also currently we use oc:fileid which is not just the numeric fileid but its %08d of the fileid followed by the instance id like 00000012oc12abcd and we just use the fileid part of it.
Because with snowflake ids the instance id is already encoded in the fileid, does it make sense to have a new DAV property nc:fileid which is just the snowflake id?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review 3️⃣ Waiting for reviews type: enhancement 🚀 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants