folium icon indicating copy to clipboard operation
folium copied to clipboard

Process JS code separately [draft]

Open Conengmo opened this issue 1 year ago • 5 comments

Testing out how we can use the concept of the new JsCode class, which was added in https://github.com/python-visualization/folium/pull/1848. Idea is that we no longer have to manually put Javascript code in templates.

Problem before was that a Javascript-function-as-Python-string would get transformed into a Javascript string. Because of that we had to put those functions into the template manually.

Now we have a JsCode class that we can use to mark strings as Javascript code. When putting stuff in templates, we can check for that class and prevent adding string quotes.

I looked into using a custom JSON encoder with the json module, but that turns out to be awkward, because what we're producing is not actually valid JSON.

So I've opted to make a custom filter like the tojson filter we use a lot, but now allowing for Javascript code. To register this filter I had to subclass Template.

Using it means:

  • Importing the Template class from folium.template
  • Wrapping any arguments that are JS functions with JsCode.optional_create()
  • Stop using parse_options.
  • Using {{ options|tojavascript }} filter in the template.

I've now applied this new workflow on the LayerControl, MousePosition and Realtime classes.

Some smaller included changes:

  • Make JsCode a subclass of str, that makes it easier to put it in a template without additional needed code.
  • Arguments that are JS functions can be either string or JsCode, I figured that's more user-friendly then requiring JsCode.
  • The MousePosition example with the custom formatter no longer works with the trailing ; character, so that's a breaking change.

If this seems like a good change I'll add tests as well.

Question is whether the added complexity of the new code weighs up to the more straightforward way in working with Javascript code.

Conengmo avatar Jan 03 '24 10:01 Conengmo

Maybe I'll split off some concepts in this PR into separate PRs:

  • Make JsCode a subclass of str.
  • Allowing str type arguments in RealTime.

Conengmo avatar Jan 03 '24 10:01 Conengmo

Slightly related: what are the recommended methods of string interpolation within JsCode? One issue I ran into: f-strings don't work so well for JS code since there will be too many curly braces to escape. I ended up using "%s" but there might be cases where some of the "%" needs to be escaped as well..

prusswan avatar Jan 04 '24 07:01 prusswan

what are the recommended methods of string interpolation within JsCode?

If escaping all those curly brackets is awkward, you could consider breaking up the string, and only apply the formatting on the part where you want to insert a variable. Example where I insert a custom id:

source = ("""
    function(responseHandler, errorHandler) {
        let geojson = {
            'type': 'FeatureCollection',
            'features': [{
                'type': 'Feature',
                'geometry': {
                    'type': 'Point',
                    'coordinates': [Math.random(), Math.random()]
                },
                'properties': {
                    'id': """ + f'{my_id}' + """
                }
            }]
        };
        responseHandler(geojson);
    }
""")

Conengmo avatar Jan 04 '24 09:01 Conengmo

I also tried adapting the json encoder for this and I came to the same conclusion. I did consider your approach as well (using a separate to_javascript jinja filter), but I thought it would be to big a change. It is cleaner though from the perspective of a plugin writer.

On a side note will this pull request need to be finalized before you will make a new release containing the realtime plugin? I need the realtime plugin for a work project. I can make my own release, but I'd prefer to stick to using the official folium releases.

hansthen avatar Jan 18 '24 18:01 hansthen

Thanks for your comments @hansthen.

I thought it would be to big a change. It is cleaner though from the perspective of a plugin writer.

Well said, and I agree. I don't think this change is worth it. Maybe if we encounter this more in the future it could become worth it, but for now it's too much additional complexity for too little saved logic.

On a side note will this pull request need to be finalized before you will make a new release containing the realtime plugin?

No, that's not necessary. I split off a small idea from this PR into a separate PR (https://github.com/python-visualization/folium/pull/1862), maybe that would be nice to get processed before doing a release. I'll also check out https://github.com/python-visualization/folium/pull/1861. Afterwards, let's do a release.

Conengmo avatar Jan 21 '24 10:01 Conengmo