[PATCH 1/5] static: added JS Cookie Library to get csrftoken for fetch requests

Raxel Gutierrez raxel at google.com
Wed Jul 21 03:43:05 AEST 2021


Hi,

> The first thing I wonder when looking at the description above is "why
> wasn't this needed before"?
>
> There are no existing users of document.cookie in patchwork.  Is the
> point that all existing code uses {% csrf_token %} in forms generated
> by the server instead of dynamically generated requests?  If so, makes
> sense.

New patch commit message makes it more clear :)

> How do we decide between this going in lib/packages/ versus
> htdocs/js/?
>
> (That's a genuine question --- I don't understand patchwork's current
> split.  Is the idea that lib/packages/ is supposed to contain a
> package with a README and htdocs/js/ is supposed to contain symlinks
> to there?)

I'm not sure what the htdocs directory is intended for exactly. It
seems like static files. But this is a good question to ask given that
there seems to be overlap. I'm not sure what exactly differentiates
the intended contents of these two directories. As for this patch,
patch-list.js follows the current pattern of the bundle.js file being
added to htdocs/js.

> > @@ -21,6 +21,7 @@
> >    <script src="{% static "js/bootstrap.min.js" %}"></script>
> >    <script src="{% static "js/selectize.min.js" %}"></script>
> >    <script src="{% static "js/clipboard.min.js" %}"></script>
> > +  <script src="{% static "js/js.cookie-2.2.1.min.js" %}"></script>
>
> Should this use an unversioned URL like the rest of these?

The version is specified in the htdocs/js README, so perhaps it would
be nice to stay consistent and remove it? However,
jquery-1.10.1.min.js would continue to break this consistency, so I'm
not sure about what's right.

> Also, how do we decide between putting this in base.html (i.e., all
> pages) versus specific pages making requests that need a csrf token?
> The script is small enough that it shouldn't make a difference, but
> asking anyway because I am curious.

Personally, I think adding those dependencies in the base.html makes
it easier later when creating new templates as you don't have to
remember which one to add as the number of them continues to grow.
Also, I think following the mindset that more content should be
handled dynamically client-side with JavaScript means that more
templates will be using the library. Actually, I think all of them
should eventually be using it.


More information about the Patchwork mailing list