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

fix error "initializeDataTables is not a function" when a notebook is reloaded #160

Merged
merged 6 commits into from
Mar 11, 2023

Conversation

mwouts
Copy link
Owner

@mwouts mwouts commented Mar 8, 2023

Sometimes when a notebook is reloaded from disk, the tables don't load, and in the console I find a message window.initializeDataTables is not a function.

This PR intends to postpone the table initialization when this occur, to let some time for the init script to load (that script is located in another section of the HTML document).

@fwouts , do you see a better approach? Ideally I would declare a dependency of (the rendered version of) datatables_template.html on (the rendered version of) initialize_offline_datatable.html but I have not idea how to do that...

@fwouts
Copy link
Collaborator

fwouts commented Mar 9, 2023

I suspect the problem is that <script type="module"> code isn't evaluated immediately like <script>, it's automatically deferred (like <script type="module" defer>). It appears that cannot be disabled.

You can see an example with the following file:

<!DOCTYPE html>
<html>
  <body>
    <script>
      console.log("Hello from regular script 1");
    </script>
    <script type="module">
      console.log("Hello from module");
    </script>
    <script>
      console.log("Hello from regular script 2");
    </script>
  </body>
</html>

The output here (in Chrome at least) is:

Hello from regular script 1
Hello from regular script 2
Hello from module

However as far as I understand, a deferred script is still guaranteed to run before $(document).ready() (which I believe is equivalent to the browser's native DOMContentLoaded event), so I'm not sure why it's possible to get this bug in the first place!

I've attempted to fix this in catch_retry...fwouts:itables:catch_retry by using a promise. I haven't tested this code though, as I'm not sure how to do that :) There could be a bug or two in there.

Is there a contributing guide somewhere?

@mwouts
Copy link
Owner Author

mwouts commented Mar 9, 2023

Yes - I am not sure how this happens but it does really happen (Jupyter Lab in Chrome under Windows). I have given a quick try at the promise approach and it seems to work really well! Thanks Francois.

Is there a contributing guide somewhere?

You mean a guide on how to setup a dev environment and do some tests? Not at the moment but that is certainly a great idea. I'll take a note of the tests I will be doing, this way we can draft an initial guide...

@mwouts mwouts changed the title catch "initializeDataTables is not a function" and retry later on fix error "initializeDataTables is not a function" when a notebook is reloaded Mar 9, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2023

Codecov Report

Merging #160 (05111c2) into main (c6e7388) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #160      +/-   ##
==========================================
+ Coverage   96.44%   96.49%   +0.04%     
==========================================
  Files          23       23              
  Lines         732      742      +10     
==========================================
+ Hits          706      716      +10     
  Misses         26       26              
Impacted Files Coverage Δ
itables/sample_dfs.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fwouts
Copy link
Collaborator

fwouts commented Mar 10, 2023

Perfect, the contributing guide is typically named CONTRIBUTING.md. Looking forward to it!

@mwouts
Copy link
Owner Author

mwouts commented Mar 10, 2023

Perfect, the contributing guide is typically named CONTRIBUTING.md. Looking forward to it!

Thanks! I have a draft at docs/contributing.md - this way it gets included in the documentation as well. Let me know what you think.

@fwouts
Copy link
Collaborator

fwouts commented Mar 10, 2023

It looks good! I hadn't seen docs/developing.md, that was basically what I needed :)

@mwouts mwouts merged commit 7c14e3f into main Mar 11, 2023
@mwouts mwouts deleted the catch_retry branch March 11, 2023 17:18
@mwouts mwouts mentioned this pull request Mar 11, 2023
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