Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decide how we want versioning via script embed to work #50

Closed
osdiab opened this issue Jan 28, 2021 · 23 comments
Closed

Decide how we want versioning via script embed to work #50

osdiab opened this issue Jan 28, 2021 · 23 comments
Assignees

Comments

@osdiab
Copy link
Contributor

osdiab commented Jan 28, 2021

After doing #49 , let's decide where it should go. Right now no way for user to set the version they're using, it's totally in our control - we should potentially expose that

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

Case studies for Pinterest pin button, Facebook Like button

Pinterest: https://developers.pinterest.com/tools/widget-builder/

  • script tag
  • and accompanying anchor tag with data-pin-* attributes for customization
  • no control over version using

Facebook: https://developers.facebook.com/docs/plugins/like-button/

  • script tag
  • div tag with fb-like class
  • configuration via data-* attributes
  • query-like parame (that is actually in the url hash - maybe for caching reasons?) to control SDK version

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

I think allowing users to control the version is less developer-antagonistic; but for nonprofits who don't have the resources to spend time on this kinda tech stuff, i think the common case should be more like pinterest's flow of by default just using the latest version (barring breaking changes).

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

The other thing I think would be useful, is to have some avenue for configuring the button on behalf of nonprofits, who once again may not want to spend the time setting all these config values; as we have a lot more parameters than either the facebook like/pinterest pin buttons; perhaps these aren't great examples of experiences to emulate, if what we're doing is something more complex, more akin to, say, the intercom/zendesk chat bubbles with branding options.

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

Intercom, as an example, seems to do all the config on their website, and probably the embed calls back home to get that data; we can potentially do something similar.

the approach I chose at the moment with our cloudflare worker was a client-specific URL, which I figured we could long-term inject configuration from the edge worker, leading to a more performant experience; but perhaps that's too standards-breaking/counterintuitive.

https://www.intercom.com/help/en/articles/178-customize-the-intercom-messenger-basics

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

Maybe we can just make these preconfigured URLs for nonprofits on a bespoke basis, but for the self-serve flow, just make the copy-paste flow have an optional version parameter that accepts something like standard semver ranges, or maybe just a major version; by default either not specify a version, or pin a major version.

Re no version specified vs pinning, the upside of no version specified for us is that we can update existing clients for them. The downside of pinning a major version is that we can't force update clients if we go down that road; we can avoid breaking existing sites when a major version comes by still "remembering" the version we were serving, say, the domain that the request came from, trusting the Origin header; so that if we make a breaking change, we can still keep serving the older one until we've ensured it works on all the client websites/reached out to their teams.

or we can just try hard to not make breaking changes, but given that this widget is in its infancy, i think we should be optimizing for making it powerful and polished over backwards compatibility for now.

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

@jake-nz @markulrich what do you guys think?

@markulrich
Copy link
Member

Agree ability for us to configure options for a given client is pretty important, especially as hopefully people will eventually be able to that from their Every.org admin dashboard - what do you think about https://github.com/everydotorg/every.org/issues/7297?

As for a version= for more developer friendly approach, created https://github.com/everydotorg/every.org/issues/7296 but maybe we don't need this?

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

i'm gonna move that issue here since jake can't see it if it's in our main repo lol

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

Posted by @markulrich : Today, we recommend people load script from URLs like https://assets.every.org/donate-button/CLIENT_ID/donate-button.js which I find a little confusing - can we instead make this an optional query parameter?

So change to https://assets.every.org/donate-button.js?clientId=CLIENT_ID

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

I would describe it rather - we in theory would recommend them that, but our README is outdated :)

I think the lack of copypastability of the API i chose tilts strongly in favor of that API - i'm happy to go with that! and we retain the potential for supporting specific version pinning if we add support for a version parameter. I do wonder why Facebook chose to put it in the hash instead of the query parameter, but to my knowledge i think query param should be fine.

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

yeah idk why, it's pretty common for people to use query params to cache bust (for instance, stackoverflow does on its static js and css assets). I'm gonna assume that it's fine for now. So yeah lets go for a static URL that's just donate-button.js, for now lets not implement specific version stuff, and if a client query parameter is present, we can customize how it's provided (which version, and other config once we implement that).

we should be careful about caching here - while we want people to load from cache for perf, we dont want caching to cause people to be stuck on old versions, so i'll make sure our caching policy's on cloudflare's side makes sense. once it's implemented we can make sure to edit the README to reflect this properly. I'll leave the logic for the existing URLs to keep working so that old clients dont break, but in the future, it would be a query param.

@osdiab
Copy link
Contributor Author

osdiab commented Jan 28, 2021

one thing that might need to change from above is that we have a folder of stuff, not just a single js file, that does chunk splitting, so rather than donate-button.js it'd be more like donate-button/*. we can change the build pipeline to be a single bundle and disable code splitting, but i don't feel as strongly about that.

Also, since there's only 5 registered slugs at the moment, we can avoid breaking those by just specifically hardcoding urls with those prefixes to follow the old path.

@jake-nz
Copy link
Contributor

jake-nz commented Feb 1, 2021

If it's not too tricky, I think we could give developers 3 options:

  • Minor version pinned, for those who want full control: donate-button.js?version=2.3.4
  • Major version pinned, for those who want latest non-breaking: donate-button.js?version=2
  • Managed, for those who can't easily change their code: donate-button.js?version=CLIENT_ID

Perhaps that last option isn't really needed though, 'Major version pinned' would probably serve that use case too

@jake-nz
Copy link
Contributor

jake-nz commented Feb 1, 2021

Here's my suggestion for a simple but flexible experience for donate-button users, including versioning and config

  • Using URLs like donate-button-2.js and donate-button-2.3.4.js allows us to just serve these as normal files
  • Making the donate button code an <a> tag means it's failsafe - if the script has not loaded or does not load, the donate button will link to the nonprofit's every.org donate flow.
  • We can parse the nonprofit's slug from the <a> href and also show once or monthly based on the URL.
  • (In future) The config for tiers, images, copy etc could default to what that nonprofit's has set on every.org. Whether it's their own site or someone else supporting them with a donate button the experience is the same for users.

Include the donate-button script on your page once, ideally right after the opening <body> tag.

<script async defer crossorigin="anonymous" src="https://assets.every.org/donate-button/donate-button-2.js"></script>

Add one or more donate buttons within your page.

Show one-time donation flow
<a href="https://www.every.org/reeflegacy/donate?frequency=ONCE" data-every-button>
  <img src="https://assets.every.org/donate-button/donate-button.png" />
</a>

Show monthly donation flow
<a href="https://www.every.org/reeflegacy/donate?frequency=MONTHLY" data-every-button>
  <img src="https://assets.every.org/donate-button/donate-button.png" />
</a>

For experts:

  • If you'd like control over minor version updates you can specify the full version number you want to use. E.G. donate-button-2.3.4.js
  • Swap out the content of the <a> tag to make a custom button
  • Configure advanced options by adding another <script> tag after the first:
<script defer>
  everyMonthWidget.setOptions({
    ... // See below for configuration options
  })
</script>

@osdiab
Copy link
Contributor Author

osdiab commented Feb 1, 2021

my thoughts on the above:

Re doing it via static files - if we did that then the likely result is that developers would use donate-button-2.4.3.js and then would stay on that version indefinitely, since most people don't go in and update dependencies; good from developer control perspective but prevents us from being able to push updates to clients, so i'm not a huge fan of that.

Re granularity of specification - i think we should assume by default that nonprofits will not be updating their versions, most are not technical organizations and so I think we ought to operate under the assumption that the typical implementation will basically be set in stone for a very long time. for that reason I think we should push for just donate-button.js with no version specification and only add version specification if it's really necessary/we are pushing some very breaking change.

with that in mind i'd prefer just exporting a donate-button.js and an optional version query param or suffix to the filename that for now would only allow specifying a major version, only exposed on the Github README, to encourage people to go with a "set it and forget it" approach so we can ease the support load of dealing with antiquated versions (the current onlyfurus issue also comes from using an old version). either way it wouldn't be a simple file, it'd be logic in the cloudflare worker to decide what to serve.

Re donate-button being an <a> tag, I agree with the choice for different reasons as in #1 . the data-every-button tag is a good idea! whatever we do i think it should remain explicit, though. Also if it's not explicitly hooked up by JS we'd need to either provide an init function of sorts, or listen for changes to the DOM for matchign elements to get included so we can hook into them.

@osdiab
Copy link
Contributor Author

osdiab commented Feb 1, 2021

i also realized now that we can just make it donate-button.js and no dealing with folders - when the project is built, the compiled JS refers to absolute URLs, not relative ones. so so long as the full bundles are in place as the build script expected them, they should still work - so a flat JS export is fine. i'll go wtih assets.every.org/donate-button.js with teh query params, simplest

@jake-nz
Copy link
Contributor

jake-nz commented Feb 1, 2021

Sounds good, I think we're basically on the same page here.

Versioning
I was thinking we'd tell developers to specify a major version like donate-button-2.js and we'd only ever bump this major version if we're truly making breaking or fundamental changes. I worry that just donate-button.js would make things a bit tricky if we did create a whole new version. Specifying a full version like donate-button-2.3.4.js would be extra for experts and not what we suggest in the docs.
This setup would also align with our versions published to NPM (#48)

Serving
I'll defer to you on how to serve it, but what I had in mind was that we'd build a new version bundle like donate-button-2.3.4.js and replace also donate-button-2.js. So now donate-button-2.js and donate-button-2.3.4.js both contain version 2.3.4. Of course, we'd need cache-busting, but it would probably be simpler than using Cloudflare workers.

  • I'm totally happy to either include the major version number or no version at all in the default bundle URL.
  • I'm also happy whether we use plain file serving or serve via Cloudflare workers.

@osdiab
Copy link
Contributor Author

osdiab commented Feb 1, 2021

yeah, now that i think of it i think that makes sense jake! I still think we'd want the cloudflare worker to basically alias semver versions to specific bundles instead of including them in the github repo, though maybe we can make it work with symbolic links? not sure if github pages supports that, will look into it!

@osdiab
Copy link
Contributor Author

osdiab commented Feb 1, 2021

symlinks dont work lol. but we can still do the worker

@jake-nz
Copy link
Contributor

jake-nz commented Feb 2, 2021

Yeah, good point. We don't really want all those in the repo

@osdiab
Copy link
Contributor Author

osdiab commented Feb 2, 2021

So yeah after having played around with the code a bit more, we can do all this kinda semver features, but there needs to be other work first before we can do that dynamically, so I think we should keep it simple for now and not do it to that much detail.

I think we should only support major versions specification for now, and just manually update the available versions. so routes like /donate-button/v2.js, that's all (for why that instead of donate-button-v2.js, see below). What that means is for now we can just tell onlyfurus to use /donate-button-v2/donate-button.js, that would work the same way, the only point of doing this stuff when the functionality is that blunt, is to make the URL shorter, which is pretty minor, esp since this is not something visible to users.


The main issue with more sophisticated semver matching is that the cloudflare worker needs to know what semver versions exist on Github Pages, so if you specified v2 and we wanted to resolve that to 2.4.3, there needs to be some way for the cloudflare script to get the list of available versions (actually comparing what you inputted to such a list is simple). As of now that capability doesn't exist, github pages is a totally separate thing from the Cloudflare system and they don't really communicate.

One way this could be done is by generating the list of versions and saving it as a JSON file in the workers code on deploy; another is to post the current list of versions to Cloudflare KV on deploy; alternatively, we could just generate a file that lists out the versions when Github Pages deploys like discussed here, and routinely cache that list so that we don't have to fetch it every time a request comes in.

a couple other changes that would make the implementation simpler:

  • instead of something like assets.every.org/donate-button-v2.js, it would be nice if it were assets.every.org/donate-button/v2.js. This is because we've configured the cloudflare worker to trigger on any route to /donate-button/*, but if we didn't do that it would also intercept requests to the actual bundles at /donate-button-v2/*, which we don't need it to do. Cloudflare Workers only allows super blunt globs, you can't do a negative glob or some kinda regex thing, so this is easier to work with.
  • it would also be nice to change the bundles to serve from a subdirectory like /dist/donate-button/2.4.3, for cleaner isolation from anything else we might do with this domain.

@osdiab
Copy link
Contributor Author

osdiab commented Feb 3, 2021

Thought a bit more, I think that semver is gonna be rough to support through URLs when you take into account caching - the client might expect an older minor/patch version, and if all the files aren't being served from the same directory, then it will cause failures on older clients.

I have come around to just serving with all the files in directories with the major version in the name (PR at #65 ), and perhaps only doing something with the cloudflare worker for custom client bundles if necessary, but no longer mentioning that.

@osdiab osdiab closed this as completed Feb 10, 2021
@osdiab
Copy link
Contributor Author

osdiab commented Feb 10, 2021

we went for assets.every.org/dist/donate-button/0.2/index.js where the 0.2 changes to major version only once we hit v1. We keep the old built files so that old clients can still run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants