Use static templatetag to remove hardcoded paths #30

tubaman · 2016-04-25T21:00:04Z

These hardcoded paths cause issues. For example, if I want to host the
site at a different root path instead of '/', I have to edit these.

Also, using the static template tag in the handlebars source allows me
to move the site around without having to install node/npm/gulp and
rebuild the assets.

This is much better than my last attempt because I actually made the change in the handlebars source rather than the generated files.

Also, I realized that we get some DRY here by being able to remove staticUrlRoot from the handlebars config.

ossanna16 · 2016-04-26T15:57:40Z

Thanks @tubaman! @jtauber @paltman Can this be merged?

paltman · 2016-04-26T17:39:32Z

No.

These are source handlebars templates and are not processed by Django. They are part of the gulp static build system.

paltman · 2016-04-26T17:40:36Z

Reopening because i just reread the PR description and it seems you were aware of this but doing something I hadn't expected. Let me think about this.

tubaman · 2016-04-26T18:48:26Z

On Tue, Apr 26, 2016 at 10:40:37AM -0700, Patrick Altman wrote:

Reopening because i just reread the PR description and it seems you were aware of this but doing something I hadn't expected. Let me think about this.

Ok, let me know if you'd like to discuss on email/IRC/whatever.

Ryan

ossanna16 · 2016-04-26T18:50:37Z

@tubaman It would be great to discuss this in our Pinax Slack, if you don't mind joining (http://slack.pinaxproject.com). Thank you! :)

paltman · 2016-04-27T12:55:54Z

I'd prefer to have the discussion here for documentation purposes as we get a lot of questions about the static process so having a history of conversations related to PRs, I think is better preserved here.

paltman · 2016-04-27T12:57:57Z

@tubaman The most immediate problem I have with {% static %} in the static/src/* files is that those files are not processed by Django at all. It was a intentional decision to separate the static build process from Django so that things can be more decoupled.

I am assuming you have tested this and the {% static bit gets ignored by the handlebars processing? I would think that would cause an error.

tubaman · 2016-04-27T16:30:06Z

On Wed, Apr 27, 2016 at 05:57:58AM -0700, Patrick Altman wrote:

@tubaman The most immediate problem I have with {% static %} in the static/src/* files is that those files are not processed by Django at all. It was a intentional decision to separate the static build process from Django so that things can be more decoupled.

I totally agree that the static static build should be separate from
Django so that folks like me don't have to install npm/gulp/etc to
use the starter projects.

However, there are touch points between Django and the static assets.
For example:

Where the static assets are located(STATIC_URL in settings)
Where the assets are served from(file system, CDN, etc)

Django has really nice facilities to enable us to easily manage all
that. Pinax should take full advantage of it.

In this discussion I realized that part of my change didn't make it into
the PR so I've fixed up the PR to include that change. Namely, we don't
need staticUrlRoot in the gulp config anymore since it's already defined
in settings.py we're using the static templatetag now. DRY FTW!

I think this whole discussion boils down to this: Which do we care
about more, decoupling or DRY?

I am assuming you have tested this and the {% static bit gets ignored by the handlebars processing? I would think that would cause an error.

Yup, I've tested it but running npm run build and the _scripts.html
and _styles.html got generated correctly. I've added a new commit to
the PR which is the result of me running npm run build.

Since handlebars runs first before the Django template is processed, it
works fine since "{% ... %}" isn't a handlebars expression. Also the
only "{{ ... }}" expression in those files is meant for handlebars,
not Django.

Aside: If there were a "{{ ... }}" expression meant for Django, we'd
have to figure out a way to escape those curly braces so handlebars
doesn't process them by mistake. However, for now we don't have that
problem.

Thanks for all the work. Regardless of which way this goes, Pinax is
hugely useful to me.

Ryan

tubaman · 2016-05-25T15:41:37Z

Any further thoughts on this?

ossanna16 · 2016-05-26T15:15:35Z

@paltman ^

tubaman · 2016-09-03T19:10:53Z

Just checking in. Any thoughts on this?

brosner · 2016-09-08T00:25:34Z

I came across this independently while optimizing static assets on a project. I am in favor of this change, but I wonder if we could do more. The hashing / manifest during the gulp build might be better served by Django's staticfiles:

https://docs.djangoproject.com/en/1.9/ref/contrib/staticfiles/#manifeststaticfilesstorage

This will hash every file for proper caching by CDNs and browsers. This may be better served by some Node package integrated into the pipeline, but no solution has been presented for it yet. I'm open to that.

I think this PR is a good first step. I will make some comments in the code to get this ready for merge.

brosner · 2016-09-08T00:49:27Z

static/src/hbs/_scripts.hbs

@@ -1 +1,2 @@
-<script src='{{ assetPath "js/site.js" }}'></script>
+{% load staticfiles %}


I'd rather see this line merged with next line. Same with _styles.hbs.

So like this?

{% load staticfiles %}<script src='{% static "{{ assetPath "js/site.js" }}" %}'></script>

Do you just want it to be a one-liner for some reason? I've typically seen Django template loads on separate lines.

Correct. The reason is that when this template is rendered by Django it won't have leading newlines.

Ok, I've pushed those as one-liners.

These hardcoded paths cause issues. For example, if I want to host the site at a different root path instead of '/', I have to edit these. Also, using the static template tag in the handlebars source allows me to move the site around without having to install node/npm/gulp and rebuild the assets. Also remove staticUrlRoot since we don't need it anymore. It's already defined as STATIC_URL in settings.py. DRY FTW!

tubaman · 2016-09-08T22:05:21Z

@brosner I also like the idea of handling static assets in Python. It just seems like the tools are often weaker than those available in javascript.

tubaman · 2016-09-29T00:52:46Z

Just a note. Since we're using less, we'll always need some tool to compile that into css. I think the right thing is to have some of the pipeline in javascript(node, gulp) and some in python(django, etc). The question is: what parts of the pipeline should be in javascript and what parts are in python?

paltman · 2016-10-19T13:06:55Z

I'm am working on something that I think will address this and borrow heavily from the ideas here though it make make it hard to just merge this as well. Either way, I appreciate your contributions @tubaman, and I've come around to your way of thinking. :)

paltman · 2016-10-19T13:24:25Z

So in my effort to solve #45 I ended up simplifying things so there is no longer a need to have includes generated at all since per @brosner's recommendation we just let Django do the hashing. I originally avoided this idea because it used to be the case you had to add a third party library and do some configuration to get this hashing feature (the name of this popular library is slipping my memory right now). It's cool to see it added to Django and has sane defaults so that just a single line in the settings.py gets it working.

The work I have done for #45 can be found in this compare: paltman/zero-gulp-removal@c277f05...master and I hope to get this backported to pinax projects before the end of the month (before our 16.10 distribution).

paltman · 2016-10-19T13:25:26Z

@brosner would love your thoughts on the above.

@florapdx likewise, i'd love your thoughts on the package.json scripts.

@tubaman

Closes #45 Also, this addresses the issue many had with the static includes that were generated. Many thanks to @tubaman for bring this issue to light in #30 and while this commit makes his PR unnecessary to merge, it was his work in that PR and the follow on discussion that included @brosner pointing out that Django now includes a static storage backend that will handle the cache busting hash naming of static files for us. Closes #30

paltman · 2016-10-20T13:47:12Z

@tubaman Just pushed everything. After some testing we will create releases. For now you can test your self any pinax project by using the -dev flag on the pinax CLI:

pinax start --dev account <PROJECTNAME>

tubaman · 2016-10-24T18:04:13Z

Glad to be a part of the solution. Thanks @paltman and @brosner

tubaman mentioned this pull request Apr 25, 2016

Use static templatetag to remove hardcoded paths #28

Closed

paltman closed this Apr 26, 2016

paltman reopened this Apr 26, 2016

tubaman force-pushed the zero-use_static_templatetag-second_try branch from 1cff511 to 98c4cc6 Compare April 27, 2016 16:27

brosner reviewed Sep 8, 2016
View reviewed changes

tubaman added 2 commits September 8, 2016 13:45

Run "npm run build"

d08497c

tubaman force-pushed the zero-use_static_templatetag-second_try branch from 98c4cc6 to d08497c Compare September 8, 2016 19:55

paltman closed this Oct 20, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use static templatetag to remove hardcoded paths #30

Use static templatetag to remove hardcoded paths #30

tubaman commented Apr 25, 2016 •

edited

Loading

ossanna16 commented Apr 26, 2016

paltman commented Apr 26, 2016

paltman commented Apr 26, 2016

tubaman commented Apr 26, 2016

ossanna16 commented Apr 26, 2016

paltman commented Apr 27, 2016

paltman commented Apr 27, 2016

tubaman commented Apr 27, 2016 •

edited

Loading

tubaman commented May 25, 2016

ossanna16 commented May 26, 2016

tubaman commented Sep 3, 2016

brosner commented Sep 8, 2016

brosner Sep 8, 2016

tubaman Sep 8, 2016

brosner Sep 8, 2016

tubaman Sep 8, 2016

tubaman commented Sep 8, 2016

tubaman commented Sep 29, 2016

paltman commented Oct 19, 2016

paltman commented Oct 19, 2016

paltman commented Oct 19, 2016

paltman commented Oct 20, 2016

tubaman commented Oct 24, 2016

		@@ -1 +1,2 @@
		<script src='{{ assetPath "js/site.js" }}'></script>
		{% load staticfiles %}

Use static templatetag to remove hardcoded paths #30

Use static templatetag to remove hardcoded paths #30

Conversation

tubaman commented Apr 25, 2016 • edited Loading

ossanna16 commented Apr 26, 2016

paltman commented Apr 26, 2016

paltman commented Apr 26, 2016

tubaman commented Apr 26, 2016

ossanna16 commented Apr 26, 2016

paltman commented Apr 27, 2016

paltman commented Apr 27, 2016

tubaman commented Apr 27, 2016 • edited Loading

tubaman commented May 25, 2016

ossanna16 commented May 26, 2016

tubaman commented Sep 3, 2016

brosner commented Sep 8, 2016

brosner Sep 8, 2016

Choose a reason for hiding this comment

tubaman Sep 8, 2016

Choose a reason for hiding this comment

brosner Sep 8, 2016

Choose a reason for hiding this comment

tubaman Sep 8, 2016

Choose a reason for hiding this comment

tubaman commented Sep 8, 2016

tubaman commented Sep 29, 2016

paltman commented Oct 19, 2016

paltman commented Oct 19, 2016

paltman commented Oct 19, 2016

paltman commented Oct 20, 2016

tubaman commented Oct 24, 2016

tubaman commented Apr 25, 2016 •

edited

Loading

tubaman commented Apr 27, 2016 •

edited

Loading