Add Vue and ng-vue packages #1421

tucker-m · 2021-07-03T23:30:49Z

Also adds related parts for compiling Vue components into the existing Javascript bundle.

This is a cleaned-up version of PR #1246, which was only meant to be an experiment and generate discussion. That PR had a lot of extra commits in it for testing things; this PR has only what is necessary.

Also adds related parts for compiling Vue components into the existing Javascript bundle. Signed-off-by: Tucker McKnight <[email protected]>

Signed-off-by: Tucker McKnight <[email protected]>

JonathanTreffler · 2021-07-04T09:12:25Z

Once this is merged, I can create Pull Requests that implements the stuff from #748. 👍

anoymouserver

Not sure whether this PR is considered finished, but to my unexperienced eye it seems like the VueComponents.js and *.vue files (for the vue-component occurences) are still missing.

anoymouserver · 2021-07-04T11:04:29Z

templates/index.php

            news-scroll-enabled-mark-read="Content.markReadEnabled()"
            news-scroll-auto-page="Content.autoPage()"
            news-scroll-mark-read="Content.scrollRead(itemIds)"></div>
+


This file seems to be changed accidentally.

Suggested change

SMillerDev · 2021-07-05T08:08:44Z

App build fails with

 [23:52:09] Error in plugin "webpack-stream"
Message:
    Entry module not found: Error: Can't resolve './app/VueComponents' in '/home/runner/work/news/news/js'
resolve './app/VueComponents' in '/home/runner/work/news/news/js'
  using description file: /home/runner/work/news/news/js/package.json (relative path: .)
    Field 'browser' doesn't contain a valid alias configuration
    using description file: /home/runner/work/news/news/js/package.json (relative path: ./app/VueComponents)
      no extension
        Field 'browser' doesn't contain a valid alias configuration
        /home/runner/work/news/news/js/app/VueComponents doesn't exist
      .wasm
        Field 'browser' doesn't contain a valid alias configuration
        /home/runner/work/news/news/js/app/VueComponents.wasm doesn't exist
      .mjs
        Field 'browser' doesn't contain a valid alias configuration
        /home/runner/work/news/news/js/app/VueComponents.mjs doesn't exist
      .js
        Field 'browser' doesn't contain a valid alias configuration
        /home/runner/work/news/news/js/app/VueComponents.js doesn't exist
      .json
        Field 'browser' doesn't contain a valid alias configuration
        /home/runner/work/news/news/js/app/VueComponents.json doesn't exist
      as directory
        /home/runner/work/news/news/js/app/VueComponents doesn't exist
Details:
    domainEmitter: [object Object]
    domainThrown: false

Fixes the problem of the vue components directory not existing. Also removed the demo components that were not meant to be committed. Signed-off-by: Tucker McKnight <[email protected]>

js/package.json

js/vue_components/IconLinkCompact.vue

PHP is not parsing the Vue components, so use Nextcloud's t() Javascript function instead. This commit adds t() as a global variable on the AppController so that it can be passed into components. Also move vue-loader and vue-template-compiler to devDependencies in package.json. Signed-off-by: Tucker McKnight <[email protected]>

codecov-commenter · 2021-07-12T06:21:03Z

Codecov Report

Merging #1421 (1f74993) into master (5232f1c) will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1421      +/-   ##
============================================
+ Coverage     91.93%   91.94%   +0.01%     
  Complexity      759      759              
============================================
  Files            64       64              
  Lines          2776     2769       -7     
============================================
- Hits           2552     2546       -6     
+ Misses          224      223       -1

Impacted Files	Coverage Δ
lib/Service/FeedServiceV2.php	`100.00% <0.00%> (ø)`
lib/Command/Debug/ItemList.php	`100.00% <0.00%> (ø)`
lib/Command/Debug/FeedItemList.php	`100.00% <0.00%> (ø)`
lib/Command/Debug/FolderItemList.php	`100.00% <0.00%> (ø)`
lib/Fetcher/FeedFetcher.php	`89.11% <0.00%> (+0.44%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5232f1c...1f74993. Read the comment docs.

SMillerDev

Looks fine to me now, build works and no PHP code.

tucker-m · 2021-07-15T04:20:44Z

Thanks for looking at it, @SMillerDev and @anoymouserver! I should have marked this as a draft, as I pushed it up before I really tested it thoroughly. But it's ready to go now, unless someone has any other concerns. Thanks for catching the PHP code in the Vue template. I added Nextcloud's t function for client-side translations as a global variable in the AppController, so it can be passed into any Vue components that need it.

I'm writing up a Vue component readme that describes how to add more Vue components now. I'm also documenting how to write and run Vue component tests. I'll submit that in a separate PR so that this one can be merged sooner.

Changed - Added vue and ng-vue packages (#1421) - Reimplemented relative time formatting as a filter (#1450) - Added new `news:updater:update-user` command to update the feeds of a single user (#1360). Signed-off-by: Benjamin Brahmer <[email protected]>

Grotax · 2021-08-16T16:02:52Z

@tucker-m would you be able take a look into this to get it into a working state?

I checked it myself but don't understand much, if you don't have time anymore I would start to revert the changes.

tucker-m · 2021-10-16T21:28:01Z

Hi @Grotax, @SMillerDev, and @JonathanTreffler, sorry for my absence on this issue. I was away for a while, and then managed to lose my two-factor authentication device, locking myself out of my Github account 🤦 so I didn't see the conversation here. Just got back into my account, and am looking over things now.

I think I managed to reproduce the error that was happening earlier. It seems to be because of mismatched versions between the vue loader and template compiler. I'll get back to this with a fix.

tucker-m · 2021-12-12T00:21:41Z

Just posting an update. I'm still thinking that this is something that dependabot broke. I don't get any errors if I lock the version numbers to what they originally were. I have a new commit here: tucker-m@169936e. I'll keep testing to make sure that there are no compilation problems or runtime errors.

anoymouserver · 2021-12-12T09:25:42Z

Maybe #1456 which was the only one updating one of the dependencies to its next major version.

SMillerDev · 2021-12-12T10:17:57Z

It might be that dependabot broke it, but we don't want to stay on old unsupported versions of software so we'll have to find a way to work with never versions too.

tucker-m mentioned this pull request Jul 3, 2021

Allow Vue components to be used on AngularJS pages #1246

Closed

Add Vue and ng-vue packages

7ae72ce

Also adds related parts for compiling Vue components into the existing Javascript bundle. Signed-off-by: Tucker McKnight <[email protected]>

tucker-m force-pushed the ng-vue branch from b66dca2 to 7ae72ce Compare July 3, 2021 23:42

tucker-m added 2 commits July 3, 2021 17:45

Update package-lock.json

fd7ea1e

Signed-off-by: Tucker McKnight <[email protected]>

Update changelog

b1f3b7b

Signed-off-by: Tucker McKnight <[email protected]>

anoymouserver reviewed Jul 4, 2021

View reviewed changes

Add a Vue component, remove extras

8e6ff23

Fixes the problem of the vue components directory not existing. Also removed the demo components that were not meant to be committed. Signed-off-by: Tucker McKnight <[email protected]>

anoymouserver reviewed Jul 7, 2021

View reviewed changes

js/package.json Outdated Show resolved Hide resolved

js/vue_components/IconLinkCompact.vue Outdated Show resolved Hide resolved

SMillerDev reviewed Jul 12, 2021

View reviewed changes

Grotax requested a review from JonathanTreffler July 12, 2021 09:47

Grotax added frontend impact Javascript/Frontend code enhancement labels Jul 12, 2021

SMillerDev approved these changes Jul 15, 2021

View reviewed changes

Grotax merged commit 36d304e into nextcloud:master Jul 19, 2021

tucker-m deleted the ng-vue branch July 22, 2021 04:48

Grotax mentioned this pull request Jul 24, 2021

Release 16.0.1 #1461

Merged

anoymouserver mentioned this pull request Aug 25, 2022

Port admin settings to vue #1880

Merged

Add Vue and ng-vue packages #1421

Add Vue and ng-vue packages #1421

Uh oh!

Conversation

tucker-m commented Jul 3, 2021

Uh oh!

JonathanTreffler commented Jul 4, 2021

Uh oh!

anoymouserver left a comment

Choose a reason for hiding this comment

Uh oh!

anoymouserver Jul 4, 2021

Choose a reason for hiding this comment

Uh oh!

SMillerDev Jul 12, 2021

Choose a reason for hiding this comment

Uh oh!

SMillerDev commented Jul 5, 2021

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jul 12, 2021

Codecov Report

Uh oh!

SMillerDev left a comment

Choose a reason for hiding this comment

Uh oh!

tucker-m commented Jul 15, 2021

Uh oh!

Grotax commented Aug 16, 2021

Uh oh!

tucker-m commented Oct 16, 2021

Uh oh!

tucker-m commented Dec 12, 2021

Uh oh!

anoymouserver commented Dec 12, 2021

Uh oh!

SMillerDev commented Dec 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants