django-commons/django-cookie-consent

Re-implement showCookieBar Javascript

Closed this issue · 5 comments

sergei-maertens commented

This will introduce some (pending) breaking changes, but in the long run things should be more maintainable.

Architecture rework:

  • Do not expect HTML to be passed to the function, but instead expect a selector to an HTML DOM node
    • The template should output a <template id="cookie-consent-template">...</template> somewhere
    • Provide the #cookie-consent-template selector to the function
    • Support outputting the cookie list to a {{ cookiegroups|json_script:'cookie-consent-groups' }}
    • Pass the ID of the list to the function and parse those as JSON
  • Get rid of the hardcoded .cc-accept etc. selectors, but allow providing them as options (and use the current values as defaults)
  • Set up the script as a proper module so that <script type="module"> can be used and it can be integrated in bundlers like webpack
  • Explore/set up an example for htmx support instead of using fetch ourselves
  • Check if we can get rid of the eval which conflicts with Content-Security-Policies -> optional an afterAccept hook can be added

What does this achieve

  • It should make it easier to work with page cache (keeping into account some Vary headers), see #49
  • Modern JS + possibly easier to integrate with npm-based frontend toolchains
  • Ability to not have to rely on JS at all by using htmx

More thoughts re backwards compatibility:

  • Keep existing functions (and window scope), but rename to deprecatedShowCookieBar
  • emit console.warn when legacy function is used
  • allow disabling warnings via options

This will still make it a breaking change, but allows us to introduce the new API while keeping the old one via a simple rename. Users can then start implementing the new API on their own schedule, and we remove the deprecated API in 1.0. Likely there will be a 0.5.0 release before that, to also acquire feedback from users, most notably the ones who have opened issues about this topic.

@etienned @9mido - can I approach you folks when this ready for testing? Are there any aspects in this issue that stand out to you that I should focus, e.g. integrating it in webpack or providing HTMX support?

etienned commented

@sergei-maertens Great news that you work on that. Ring me when you're ready for some tests. I'll try later to review your proposal and give you feedback if necessary.

  • ❤️1
sergei-maertens commented

Hi @etienned - I think I have a working proof of concept now in the example app, in PR #100. Probably the documentation updates are easiest to review as a user of this library, but feel free to take a look at the code and example code too.

This is by no means done, but I'm moderately happy with the approach and available escape hatches. I've been looking a bit into htmx and alpine.js too. The latter is not going to be an option because it relies on unsafe-eval and that is for our own projects where we use this library unacceptable. HTMX itself is something to investigate a bit more, but I'm not sure if it's worth the effort, so I'm parking that.

One thing I'm still debating is if we should write the script itself as typescript so that typescript users can also benefit from the type definitions, and have a small compile step to output the actual module to the static folder. Or, we just drop the sources as a package on npm and include that in our package build. @jezdez - is there precedent for publishing some parts of a django package on npm?

Then finally, if this seems like a good direction, we also need to take a good look at which parts of the code can be deprecated (and removed for 1.0).

sergei-maertens commented

I think this is now in a good enough state for people to try it out - in the next few days I will probably publish a beta version of the next release so it's easier to test.

I've added some end-to-end tests with playwright to lock in the desired behavior, so the test suite also serves as a reference of how you could/should use this.

sergei-maertens commented

I've tested this out in our own project to see if I could easily replace our non-javascript variant: open-formulieren/open-forms#3504

Could be a useful reference for others!