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

Process JS code separately [draft] #1856

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Conengmo
Copy link
Member

@Conengmo Conengmo commented Jan 3, 2024

Testing out how we can use the concept of the new JsCode class, which was added in #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
Copy link
Member Author

Conengmo commented Jan 3, 2024

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.

@prusswan
Copy link
Contributor

prusswan commented Jan 4, 2024

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..

@Conengmo
Copy link
Member Author

Conengmo commented Jan 4, 2024

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);
    }
""")

@hansthen
Copy link
Collaborator

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.

@Conengmo
Copy link
Member Author

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 (#1862), maybe that would be nice to get processed before doing a release. I'll also check out #1861. Afterwards, let's do a release.

@Conengmo Conengmo changed the title Process JS code separately Process JS code separately [draft] Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants