bdon / observable-framework-maps

static site generator for maps + static site generator for data apps
https://bdon.github.io/observable-framework-maps/
9 stars 2 forks source link

Update example-map.md to be compatible with latest Observable Framewo… #3

Closed jaanli closed 6 months ago

jaanli commented 6 months ago

…rk version

accessing FileAttachment()._url was deprecated (see https://github.com/bdon/observable-framework-maps/issues/2); this fixes it and enables reuse of the protomaps example :) Thank you @Fil for spotting this usage of the private Observable Framework API.

closes https://github.com/bdon/observable-framework-maps/issues/2

Fil commented 6 months ago

👍

Please note that ._url is not “deprecated”, and that this was not a “breaking change”. The issue is that this code was using a private property ._url instead of the supported API .url() (an async method, still valid). Since https://github.com/observablehq/framework/releases/tag/v1.5.0 you can also use .href(which is synchronous, so might be easier to work with in some cases).

jaanli commented 6 months ago

Understood! Thank you for clarifying @Fil 🙏 - I am still clearly learning the taxonomy of breaking change vs breaking change for a specific example like this one (and the classification of the change as not breaking due to accessing a private property or other unsupported part of the API :)

Would be a fun Mermaid diagram to make for all the ways things break or don't :p

Fil commented 6 months ago

Users should be confident that “breaking changes” in Framework are properly documented and traceable through major version upgrades only (per semver).

Of course this “contract” cannot extend to elements that are not part of the API. (If you push the idea to the extreme, we would otherwise not be able to make any change to our code — just imagine if someone relies on the md5 hash of the code, or on its comments.)

JavaScript does not always allow us to mark properties as “internal”; a convention is to prefix them with an underscore.

bdon commented 6 months ago

I was using the private _url before, it would be nicer to use .url() indeed. href seems to always return undefined (so the example is broken on main right now). .url() also seems to include a cachebuster (?sha=X) which the leaflet implementation seems to fail to handle correctly right now. issue opened in https://github.com/protomaps/protomaps-leaflet/issues/147

Fil commented 6 months ago

.href was introduced in framework 1.5.0. The problem should be fixed if you upgrade (https://github.com/bdon/observable-framework-maps/blob/main/package-lock.json shows 1.0.0)

bdon commented 6 months ago

Right, It's fixed now using .href. I fixed the other bug in https://github.com/protomaps/protomaps-leaflet/issues/147 which means the example at https://bdon.github.io/observable-framework-maps/ are working using only public Framework APIs. Thanks!