Initial commit to return /client to the URL include paths

[trainerbill]

Not ready to merge yet. Need to collaborate on someone more familiar with the inner Node workings. @lirantal @ilanbiala @rhutchison

[lirantal]

@trainerbill I think it would be cleaner if we we don't /public or /client in the URL. I thought the whole point was to remove it, not add it.

[trainerbill]

@lirantal The issue is the /client was being removed from the URL. So when you are making custom modules you have to include views etc using /modules/whatever/views and not the real path /modules/whatever/client/views. According the the linked issue people thought that was awkward and wanted client in the URL.

[codydaig]

I'd prefer to have /client in the url. It's so unintuitive to not have it in the URL. For the Favicon for example, if referencing it on the server side you use /modules/core/client/img/brand/favicon.ico but to reference the same file on the client side you use /modules/core/img/brand/favicon.ico

[lirantal]

I see what you're saying, it's just very odd to me that we refer to the '/client/' as part of the URL because it seems something semantic.

If it makes sense to you and others then I won't be the one to deny the change.

[trainerbill]

@lirantal Honestly I say that we add the /client back so that development is easy. It should reference the absolute path for clarity. Then we use this:

https://www.npmjs.com/package/gulp-ng-templates

which will minify the html and put them all the views in a JS file so that they are loaded directly. This would significantly improve performance as well.

@rhutchison thoughts?

[lirantal]

I don't want to depend on the templating solution though because others may
not choose to use it either.

Let's hear what others have to say...
On Aug 5, 2015 7:43 PM, "trainerbill" [email protected] wrote:

@lirantal https://github.com/lirantal Honestly I say that we add the
/client back so that development is easy. It should reference the absolute
path for clarity. Then we use this:

https://www.npmjs.com/package/gulp-ng-templates

which will minify the html and put them all the views in a JS file so that
they are loaded directly. This would significantly improve performance as
well.

@rhutchison https://github.com/rhutchison thoughts?

—
Reply to this email directly or view it on GitHub
#758 (comment).

[codydaig]

@rhutchison Didn't you have another solution somewhere about caching the templates on load?

[trainerbill]

I think we should merge this to keep consistency throughout the app. The sooner the better since it touches so many files.

[lirantal]

@trainerbill with reagrds to your comment on /public folder

/public is still being removed. Should we take this out as well? I kinda like /public not being there and looking directly for /lib
what does it mean that /public is being removed? I actually say that if we are going for explicitness then just like we made sure that /client/ is part of the part then so should be /public

WDYT?

[trainerbill]

@lirantal My initial thought is to leave the /public being removed as that is how most other frameworks work I think. It is usually assumed that /public is the root of the pubically available resources and that you don't have to include /public when using them. IMO.

Honestly would like some others to weigh in on this.

[lirantal]

@trainerbill I'm voting for making /public/ as part of the path because /client/ is also part of it, so it makes sense to keep it consistent.

But I agree, we need more eyes.

[codydaig]

IMO, I like the /public.

[trainerbill]

@codydaig You want the /public in the url or you want to remove it like it is currently doing?

[codydaig]

@trainerbill I would like to see it in the URL, not how it's been removed for a while.

[lirantal]

then me and @codydaig are on the same page.

[trainerbill]

@lirantal @codydaig I have this PR up to date and ready. I honestly think that /public can be removed as that is how most frameworks handle it... The root of the server is /public from express.js:

// Setting the app router and static folder
  app.use('/', express.static(path.resolve('./public')));

Can we merge this PR and have a separate discussion on /public?

[lirantal]

OK with me.

[codydaig]

Sounds good to me.

[lirantal]

Good then.

[codydaig]

@trainerbill Do you want to do the PR for /public, or do you want me too?

[trainerbill]

@codydaig Honestly I don't think we should remove the /public as the root of the client side server is defined there. Most frameworks do the same thing and definitely don't include the /public in client side includes. I will leave it up to you guys. @lirantal

[lirantal]

I don't have a stronger argument than my previous one which is to be consistent in the URLs, meaning we either "remove the magic" and make all URLs explicit, or we don't. At the moment /client/ is explicitly there but /public/ is not.

Maybe someone else will have more to contribute, but anyway if we want to pursue this then this requires a new issue, let's not hijack this PR too much.