Skip to content

Conversation

@ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jun 25, 2025

  • Preparation for [RFC] export of vue components #1809
  • Currently @nextcloud/upload has mixed pure API, ui functions (dialogs) and a vue component
    • Also, it has some circular dependencies
  • This PR refactors file/module structure:
    • . - pure uploading logic, kept in place
    • dialogs - UI related functions that use Vue only internally and will be moved to @nextcloud/dialogs in the next major
    • vue - exported Vue component (that can only be used in a specific Vue version and will be moved to @nextcloud/vue in the next major)
Before After
image image

Detailed explanation

Details

image

image

image

image

image

image

@ShGKme ShGKme requested review from skjnldsv and susnux June 25, 2025 13:22
@ShGKme ShGKme self-assigned this Jun 25, 2025
@ShGKme ShGKme added the 3. to review Waiting for reviews label Jun 25, 2025
@ShGKme ShGKme added the technical debt Technical issue label Jun 26, 2025
@ShGKme ShGKme changed the title refactor: separate files by core, ui, components sub-folders refactor: split modules (files) by core, ui and components sub modules to prepare for library splitting Jun 26, 2025
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

LGTM

@susnux
Copy link
Contributor

susnux commented Jun 26, 2025

Makes sense in v1.
For v2 we would then need to revert it.

@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 26, 2025

For v2 we would then need to revert it.

Why do we need it? The only purpose of the PR is to prepare for v2.
And if we revert, then it's complicated again to move these parts out from the library again.

@susnux
Copy link
Contributor

susnux commented Jun 26, 2025

And if we revert, then it's complicated again to move these parts out from the library again.

Because there will be only the core left.

@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 26, 2025

Because there will be only the core left.

I don't understand it in the context of "And if we revert, then it's complicated again to move these parts out from the library again.".

And yes, only the core part is left. That is the purpose of the PR, why is it a problem that requires revert to the mixed state?

With the refactoring there are 3 separated parts. 2/3 can be easily removed, without any changes in the remaining one. Without it, we have mixed modules using each other, including different parts sometimes in a single file.

The only purpose of the PR is to go to v2. If v2 requires reverting the "prepare for v2", when we should close it.

@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 26, 2025

Or the only problem is that the "core" as a folder exists without other folders?

If that is the case, we can just move its content to the parent.

@ShGKme ShGKme force-pushed the refactor/separate-files-by-sub-packages branch 2 times, most recently from 388185f to df1f5d1 Compare June 27, 2025 12:10
@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 27, 2025

  • Removed core to keep everything from core in root:
    • Then these files have no changes in v2
  • Renamed ui to dialogs (goes to the @nextcloud/dialogs)
  • Renamed components to vue (goes to the @nextcloud/vue)

@ShGKme ShGKme force-pushed the refactor/separate-files-by-sub-packages branch from df1f5d1 to 62ead94 Compare June 27, 2025 12:14
@ShGKme ShGKme changed the title refactor: split modules (files) by core, ui and components sub modules to prepare for library splitting refactor: split vue and dialogs sub modules to prepare for library splitting Jun 27, 2025
@ShGKme ShGKme merged commit 8197c23 into main Jun 27, 2025
21 checks passed
@ShGKme ShGKme deleted the refactor/separate-files-by-sub-packages branch June 27, 2025 12:28
@susnux susnux mentioned this pull request Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews technical debt Technical issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants