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

Replaced search for .kytos files with .vue #508

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HeriLFIU
Copy link

Closes #issue_number

Summary

Since we are moving from utilizing the httpvueloader to now using the vue3-sfc-loader, we must now change the napp ui file extension from .kytos to .vue. This is because the vue3-sfc-loader only works on .vue files.

Local Tests

End-to-End Tests

Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

@HeriLFIU, overall looks the draft looks good, and great the direction it's moving towards, but here a point for you to think about:

  • This is a breaking change, can we have sfc loading a custom .kytos as we land these PRs? Since that would make it gradually easier, otherwise we'd also need to land together with the NApps PRs renaming their ui/*.kytos files to ui/*.vue files.

Ultimately, I think it's OK to have officially .kytos to always be .vue in the UI dir, but let's also see if we can temporarily also still support .kytos until we ship the next version, if it's doable and not too much extra unplanned work, can you look into it to see if it's a viable option?

@HeriLFIU
Copy link
Author

HeriLFIU commented Nov 1, 2024

@viniarck I had tried to search for a method within the documentation but couldn't find anything. I can take another look, try and search for other methods, or maybe we can try and modify SFC directly if the license allows for it.

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