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

[BUG] Cannot change locale with external CDN source #880

Closed
Batalex opened this issue Aug 21, 2019 · 13 comments
Closed

[BUG] Cannot change locale with external CDN source #880

Batalex opened this issue Aug 21, 2019 · 13 comments

Comments

@Batalex
Copy link
Contributor

Batalex commented Aug 21, 2019

Describe your context

dash==1.1.1
dash-core-components==1.1.1
dash-html-components==1.0.0
dash-renderer==1.0.0
dash-table==4.1.0

Describe the bug

To change Plotly language (modebar + number format), one need to:

  1. load the localization file
  2. Set the locale config key for each graph

Unfortunately, this is problematic with Dash if you want to use the CDN link. From Dash documentation:

External css/js files are loaded before the assets.

which is fine most of the time. However, here is Plotly.js documentation on localization:

After the plotly.js script tag, add [...] the locale definition

Here is the resulting footer of adding the CDN link to external_scripts:

...
<script src="https://cdn.plot.ly/plotly-locale-fr-latest.js"></script>
<script src="/_dash-component-suites/dash_core_components/plotly-1.49.1.min.js?v=1.1.1&m=1565128596"></script>
...

The modebar is not translated in this case.

Expected behavior

Here is the footer when loading the localization file as a local asset:

...
<script src="/_dash-component-suites/dash_core_components/plotly-1.49.1.min.js?v=1.1.1&m=1565128596"></script>
<script src="/_dash-component-suites/dash_core_components/highlight.pack.js?v=1.1.1&m=1565030553"></script>
<script src="/_dash-component-suites/dash_core_components/dash_core_components.min.js?v=1.1.1&m=1565131287"></script>
<script src="/_dash-component-suites/dash_html_components/dash_html_components.min.js?v=1.0.0&m=1561057513"></script>
<script src="/assets/plotly-locale-fr.js?m=1566399183.1702719"></script>
...

The localization file is indeed loaded after Plotly.js, and it works as expected.

Also, this issue happens as well with serve_locally=False .

Suggestion

We can specify some attributes (namely: src, integrity, crossorigin) for each external script, maybe we could add an order indication.
Or perhaps the localization could be a parameter for the Dash application?

Edit: typo

@alexcjohnson
Copy link
Collaborator

TLDR: @chriddyp @Marc-Andre-Rivet do you see any problem moving external_scripts immediately after assets (which itself comes after component suites) in the script load order?


@Batalex interesting - that is certainly a problem, thanks for the report! At some point we may want to handle localization dash-wide, so it can be applied consistently and easily to all elements that show text (see eg plotly/dash-table#300) but we are a ways away from that.

There's relatively complicated ordering built into dash right now, primarily via:

dash/dash/dash.py

Lines 600 to 613 in f0b6be9

srcs = (
self._collect_and_register_resources(
self.scripts._resources._filter_resources(
deps, dev_bundles=dev
)
) +
self.config.external_scripts +
self._collect_and_register_resources(
self.scripts.get_all_scripts(dev_bundles=dev) +
self.scripts._resources._filter_resources(
dash_renderer._js_dist, dev_bundles=dev
)
)
)

Which has the effect of ordering things:

  1. Render dependencies (react and friends) via deps
  2. external_scripts
  3. Dash component bundles (first in get_all_scripts)
  4. The assets folder (not at all obvious, but these get added last in get_all_scripts)
  5. dash_renderer - I'm not sure it matters any more but this used to need to be last because the renderer bundle would immediately instantiate the renderer. It doesn't anymore because...
  6. Now there's a separate little script, customizable via app.renderer, that does this and actually sets the whole machine in motion. This is a separate piece of the html template so doesn't show up in the block above ⬆️

Now, seems like it would be more self-consistent to put external_scripts immediately after assets, and it seems like that would solve your immediate issue. But would it cause any other issues? Is there ever a reason you'd need an external script (or an asset) to come elsewhere in this ordering?

There's always the option to override the default html template, via app.index_string or subclassing app.interpolate_index (see https://dash.plot.ly/external-resources -> "Customizing Dash's HTML Index Template") to explicitly set the order exactly as you want it. And in fact that's the only reasonable way I can think of to control the load order of files in assets - along with assets_ignore so they don't get loaded twice. So if we can convince ourselves that moving external_scripts after components and assets is safe, I'd vote to just do that and not add an order field - which I imagine would be confusing to use as well as not covering assets.

@Batalex
Copy link
Contributor Author

Batalex commented Aug 22, 2019

Hum I believe Mathjax.js has to be loaded before Plotly.js so moving all external scripts after the assets is not a viable alternative

@alexcjohnson
Copy link
Collaborator

Ah, you're right - so MathJax must have the opposite bug right now, that it'll work via CDN but not assets.

It occurs to me it might not be hard to remove that restriction for MathJax - have plotly.js configure it on the first plot call rather than on script load. And in principle we could do something similar to make locales function either before or after plotly.js, though that may be a little tricky to get right in all cases.

But in the spirit of "once is never, twice is always" perhaps this tells us we'll see this problem again and we do need an order param to save everyone from custom index templates. There might be things that need to come even before React, like browser polyfills?

So perhaps something like:

  • Items 1-5 above get assigned an order 100, 200, 300, 400, 500.

  • After creating the full list we sort by order. For items with the same order value, original list order is preserved.

  • external_scripts would default to 200 but you could add an order param to override this

  • Two ideas come to mind for assets:

    • a new param assets_order would map asset names (or regexes?) to order numbers
    • incorporate the order into the file name or path - either as subfolders 150/my_early_script.js or prefixes 150_my_early_script.js

    The subfolder version may be the cleanest - it's DRY (so won't break if filenames change for example) and doesn't require renaming files (which would be annoying when bumping versions)

@Marc-Andre-Rivet
Copy link
Contributor

Marc-Andre-Rivet commented Aug 22, 2019

Items 1-5 above get assigned an order 100, 200, 300, 400, 500.

This can work. Would prefer a named convention. If additional buckets get added or if new features in Dash impact ordering, it becomes easier to have a predictable behavior between releases and to abstract away the ordering. I'd see something like bucket +/- score to define if a resource gets loaded before or after a given bucket. (e.g externals+20 or externals-20 subfolders.

e.g. initial buckets could be externals, assets, components

The subfolder version may be the cleanest - it's DRY

Agreed.


I'd be interested in seeing this solution apply to components too through some mechanism. For example, DDK requires the dash-table to be loaded before but there is no way of guaranteeing that atm and a jerry rigged solution was required. Being able to declare that it depends on the dash-table and having the server order the components accordingly would be better.

@Marc-Andre-Rivet Marc-Andre-Rivet added this to the Dash vFuture milestone Aug 23, 2019
@Batalex
Copy link
Contributor Author

Batalex commented Nov 20, 2019

It seems like since 1.5.0 and its async dependencies it is now impossible to change the locale due to plotly/plotly.js#4146

plotly-locale-fr.js?m=1574239464.9144504:1 Uncaught ReferenceError: Plotly is not defined
    at plotly-locale-fr.js?m=1574239464.9144504:1

@alexcjohnson
Copy link
Collaborator

ah good point. You can put a version of plotly.js in your assets dir, then it'll at least load synchronously - but I don't think it'd be guaranteed to load before the locales even so. plotly/plotly.js#4146 is really the solution we want.

@antoinerg
Copy link
Contributor

Order-independent loading of locales will be provided by plotly/plotly.js#4453

As for MathJax, as per plotly/plotly.js#4146 (comment), when window.PlotlyConfig = {MathJaxConfig: 'local'}, plotly.js doesn't do any Mathjax config on load so

it might make sense for Dash to always set this MathJaxConfig flag to 'local' before loading plotly.js as well.

With this change, MathJax should work the same regardless of when it is loaded.

@antoinerg
Copy link
Contributor

FYI, PR plotly/plotly.js#4453 is now merged and the associated issue plotly/plotly.js#4146 is closed.

@Marc-Andre-Rivet Marc-Andre-Rivet modified the milestones: Dash v1.x, Dash v1.8 Jan 8, 2020
@Marc-Andre-Rivet
Copy link
Contributor

@antoinerg Let us know when this is released in Plotly.js and we'll update Dash and see about adding a test for this scenario.

@antoinerg
Copy link
Contributor

antoinerg commented Jan 8, 2020

@Marc-Andre-Rivet plotly.js 1.52.0 just landed: https://github.com/plotly/plotly.js/releases/tag/v1.52.0 and it contains the fix (plotly/plotly.js#4453)!

@Marc-Andre-Rivet
Copy link
Contributor

@Batalex The new dash==1.8.0 release uses the new Plotly.js version with the fix. Please let us know if this solves your problem.

@Marc-Andre-Rivet Marc-Andre-Rivet modified the milestones: Dash v1.8, Dash v1.9 Jan 14, 2020
@Batalex
Copy link
Contributor Author

Batalex commented Jan 15, 2020

@Marc-Andre-Rivet Still not resolved on my side... I will share a small reproductible app in order to confirm the behavior.

@Batalex
Copy link
Contributor Author

Batalex commented Jan 15, 2020

Ok, given this very simple app:

import dash
import dash_core_components as dcc
import dash_html_components as html

external_scripts = ["https://cdn.plot.ly/plotly-locale-fr-latest.js"]

app = dash.Dash(name=__name__, external_scripts=external_scripts)
app.layout = html.Div([dcc.Graph(figure={}, config=dict(locale="fr"))])

if __name__ == "__main__":
    app.run_server(debug=True)

The modebar is translated as expected in 1.8.0 but not in 1.7.0.
Closing for now, as my issue is probably my own fault!

@Batalex Batalex closed this as completed Jan 15, 2020
HammadTheOne pushed a commit to HammadTheOne/dash that referenced this issue May 28, 2021
HammadTheOne pushed a commit that referenced this issue Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants