Initial Commit with Static baseURL in vue project #125

KThornley · 2018-10-11T03:37:44Z

Re Issue #124 I have done the initial changes for this configuration, However what is still outstanding is the dynamic injection of the baseUrl into the frontend at build and axios (I have currently hard coded this for my purposes). Do you have a design you would like followed for config injection into Vue?

codecov-io · 2018-10-11T04:06:48Z

Codecov Report

Merging #125 into master will increase coverage by 0.09%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
+ Coverage   64.58%   64.68%   +0.09%     
==========================================
  Files          24       24              
  Lines        2067     2067              
==========================================
+ Hits         1335     1337       +2     
+ Misses        581      580       -1     
+ Partials      151      150       -1

Impacted Files	Coverage Δ
handlers/handler.go	`85.13% <0%> (ø)`	⬆️
security/vault.go	`77.37% <0%> (+1.45%)`	⬆️

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 bcaf623...7af97d2. Read the comment docs.

michelvocks · 2018-10-11T11:01:44Z

Welcome @KThornley 🤗
Thanks a lot for your contribution!

I'm a bit busy today but will have look tomorrow for sure.
One question beforehand: Did you upgrade all NPM packages? If so, was it required? 😄
Usually we have to test the whole UI after doing that (manually).

Skarlso · 2018-10-11T11:43:05Z

I'll take a look tonight probably. If nothing comes between.

Skarlso · 2018-10-11T14:45:15Z

@KThornley Hey. There is a LOT of change in the lock file. Was there a reason for that? :)

KThornley · 2018-10-11T21:10:53Z

Sorry overeager node plugin in IDE

KThornley · 2018-10-11T21:46:23Z

Have thought about how to inject baseURl found out that building the Vue app with assetsPublicPath: './' and setting the axios client to also be relative const axiosInstance = axios.create({ baseURL: './' }) resolved it but introduced some weirdness with fontloading adding assets/css/ to the paths. stripping them in the handler was a quick fix, but there is probably a proper way to do that

Skarlso · 2018-10-12T05:27:14Z

frontend/client/app.js

-Vue.prototype.$http = axios
-Vue.axios = axios
+const axiosInstance = axios.create({
+  baseURL: './'


This means that ./ will always be the default. Shouldn't this be just /?

it will always be relative to the current base and since its a SPA that will be where ever it is served from. / would be absolute so we would have to do some trickery of setting the conditional root and I am lazy :)

Skarlso · 2018-10-14T12:11:07Z

@KThornley Alright, this is looking good. Could you please do me a favour and add in tests which use a different setting than the default and see if everything is still okay? Like, we already know that it works with / now make it work with something else. :) You know what I'm saying?

Thanks!

KThornley · 2018-10-26T07:04:14Z

Hi sorry just getting back to this, I think I can simplify this solution with just the changes to the vue project. I will revert my changes to the handler and see how I get on

Skarlso · 2018-10-26T07:17:11Z

@KThornley Cool, thanks very much! :)

Skarlso · 2018-11-01T07:31:11Z

Hi @KThornley. Do you have something on this or shall I pick it up and finish some of it?

KThornley · 2018-11-01T21:07:58Z

Hi @Skarlso I reverted the changes to handler and it works with just the vue changes and one minor path change to handler (below), seems to be that webpack adds extra /assets/css prefix to the font urls when relative paths are specified. Did you want me to close this pull request and recreate another with the squashed changes against your current master? It would probably be cleaner
e.GET("/assets/css/assets/fonts/*", echo.WrapHandler(http.StripPrefix("/assets/css/", assetHandler)))

Skarlso · 2018-11-02T17:41:19Z

@KThornley nope it's fine like this mate. Thanks very much for your effort!

Skarlso · 2018-11-05T10:28:57Z

Hi @KThornley. So I tried checking out the code and testing it a bit and when I created a pipeline this is what I've got:

vue.runtime.esm.js?a427:574 [Vue warn]: Error in mounted hook: "TypeError: this.$http.all is not a function"

found in

---> <Detail> at /Users/gbrautigam/gohome/src/github.com/gaia-pipeline/gaia/frontend/client/views/pipeline/detail.vue
       <AppMain> at /Users/gbrautigam/gohome/src/github.com/gaia-pipeline/gaia/frontend/client/components/layout/AppMain.vue
         <Root>
warn @ vue.runtime.esm.js?a427:574
vue.runtime.esm.js?a427:1713 TypeError: this.$http.all is not a function
    at VueComponent.fetchData (detail.vue?4b36:195)
    at VueComponent.boundFn [as fetchData] (vue.runtime.esm.js?a427:191)
    at VueComponent.mounted (detail.vue?4b36:133)
    at callHook (vue.runtime.esm.js?a427:2891)
    at insert (vue.runtime.esm.js?a427:4086)
    at Object.invoker [as insert] (vue.runtime.esm.js?a427:1996)
    at invokeInsertHook (vue.runtime.esm.js?a427:5860)
    at VueComponent.patch [as __patch__] (vue.runtime.esm.js?a427:6079)
    at VueComponent.Vue._update (vue.runtime.esm.js?a427:2643)
    at VueComponent.updateComponent (vue.runtime.esm.js?a427:2761)
logError @ vue.runtime.esm.js?a427:1713
vue.runtime.esm.js?a427:574 [Vue warn]: Error in mounted hook: "TypeError: this.$http.all is not a function"

found in

---> <Detail> at /Users/gbrautigam/gohome/src/github.com/gaia-pipeline/gaia/frontend/client/views/pipeline/detail.vue
       <AppMain> at /Users/gbrautigam/gohome/src/github.com/gaia-pipeline/gaia/frontend/client/components/layout/AppMain.vue
         <Root>
warn @ vue.runtime.esm.js?a427:574
vue.runtime.esm.js?a427:1713 TypeError: this.$http.all is not a function
    at VueComponent.fetchData (detail.vue?4b36:195)
    at VueComponent.boundFn [as fetchData] (vue.runtime.esm.js?a427:191)
    at VueComponent.mounted (detail.vue?4b36:133)
    at callHook (vue.runtime.esm.js?a427:2891)
    at insert (vue.runtime.esm.js?a427:4086)
    at Object.invoker [as insert] (vue.runtime.esm.js?a427:1996)
    at invokeInsertHook (vue.runtime.esm.js?a427:5860)
    at VueComponent.patch [as __patch__] (vue.runtime.esm.js?a427:6079)
    at VueComponent.Vue._update (vue.runtime.esm.js?a427:2643)
    at VueComponent.updateComponent (vue.runtime.esm.js?a427:2761)
logError @ vue.runtime.esm.js?a427:1713

Any ideas?

michelvocks · 2018-11-05T11:16:46Z

I guess it's because the variable has been removed: https://github.com/gaia-pipeline/gaia/pull/125/files#diff-70915ea35952c4bd1a7a22357344cc59L15

I'm also not sure if we use this at other places too? 😅

Skarlso · 2018-11-05T20:35:11Z

Interesting. I still see it declared but a few lines below with instance variable assigned?

michelvocks · 2018-11-06T11:56:07Z

Ah right. I'm not quite sure if already upgraded axios. axios/axios#1042 might be interesting here.

KThornley · 2018-11-06T21:20:18Z

Sorry just catching up with this, very new to Vue and didn't drop down to this function sorry, I have refactored detail to use Promise.all had a bit of difficulty with the binding so went for a variation of the function call. Good news I have a pipeline view again

Skarlso · 2018-11-07T14:48:21Z

Works, thanks man!

thornleyk added 2 commits October 11, 2018 16:02

Initial Commit with Static baseURL in vue project

7d76958

Fix Test Cases

93756da

revert lockfile

2847523

Removed hardcoded baseUrl and made it relative

b786534

Skarlso reviewed Oct 12, 2018

View reviewed changes

Skarlso added the Needs Work The PR still requires some work from the submitter. label Oct 16, 2018

revert handler changes and baseurl

9ed13cd

Skarlso added needs review and removed Needs Work The PR still requires some work from the submitter. labels Nov 4, 2018

Fix Vue Axios all and spread

7af97d2

Skarlso merged commit c3b84a9 into gaia-pipeline:master Nov 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial Commit with Static baseURL in vue project #125

Initial Commit with Static baseURL in vue project #125

KThornley commented Oct 11, 2018

codecov-io commented Oct 11, 2018 •

edited

Loading

michelvocks commented Oct 11, 2018

Skarlso commented Oct 11, 2018 •

edited

Loading

Skarlso commented Oct 11, 2018

KThornley commented Oct 11, 2018

KThornley commented Oct 11, 2018

Skarlso Oct 12, 2018

KThornley Oct 12, 2018

Skarlso commented Oct 14, 2018

KThornley commented Oct 26, 2018

Skarlso commented Oct 26, 2018

Skarlso commented Nov 1, 2018

KThornley commented Nov 1, 2018

Skarlso commented Nov 2, 2018

Skarlso commented Nov 5, 2018

michelvocks commented Nov 5, 2018

Skarlso commented Nov 5, 2018

michelvocks commented Nov 6, 2018

KThornley commented Nov 6, 2018

Skarlso commented Nov 7, 2018

Initial Commit with Static baseURL in vue project #125

Initial Commit with Static baseURL in vue project #125

Conversation

KThornley commented Oct 11, 2018

codecov-io commented Oct 11, 2018 • edited Loading

Codecov Report

michelvocks commented Oct 11, 2018

Skarlso commented Oct 11, 2018 • edited Loading

Skarlso commented Oct 11, 2018

KThornley commented Oct 11, 2018

KThornley commented Oct 11, 2018

Skarlso Oct 12, 2018

Choose a reason for hiding this comment

KThornley Oct 12, 2018

Choose a reason for hiding this comment

Skarlso commented Oct 14, 2018

KThornley commented Oct 26, 2018

Skarlso commented Oct 26, 2018

Skarlso commented Nov 1, 2018

KThornley commented Nov 1, 2018

Skarlso commented Nov 2, 2018

Skarlso commented Nov 5, 2018

michelvocks commented Nov 5, 2018

Skarlso commented Nov 5, 2018

michelvocks commented Nov 6, 2018

KThornley commented Nov 6, 2018

Skarlso commented Nov 7, 2018

codecov-io commented Oct 11, 2018 •

edited

Loading

Skarlso commented Oct 11, 2018 •

edited

Loading