Tablesaw doesn't properly initialize when script is loaded after document has loaded

[porcus]

I have gotten tablesaw to work as desired in a mock-up, but when integrating it into my app, I couldn't get it to work as expected. I traced the problem to the fact that in my app I am loading tablesaw.js script dynamically AFTER the document has loaded and then calling Tablesaw.init(). Because of this, domContentLoadedTriggered is never set to true, because it doesn't detect that the DOMContentLoaded event has fired, so it doesn't initialize properly.

I've identified a work-around for this which works for now. I just manually trigger the DOMContentLoaded event on the document after dynamically loading the script. (As expected, it works whether I call Tablesaw.init() either before or after manually triggering that event.)

Another way that I've found to make this work is to make the following changes to how domContentLoadedTriggered is used:

...
  var document = window.document;
var domContentLoadedTriggered = /complete|loaded/.test(document.readyState);
document.addEventListener("DOMContentLoaded", function() {
...
	init: function(element) {
		domContentLoadedTriggered = domContentLoadedTriggered || /complete|loaded/.test(document.readyState);
		if (!domContentLoadedTriggered) {
...

Because the DOMContentLoaded event is fired while the document is in the interactive ready state, I didn't include it in the regexes above. This way, it doesn't inadvertently initialize too early. But once the document is in a complete ready state, then it is definitely not too early for the DOMContentLoaded event to have fired, so domContentLoadedTriggered is set.

I hope you'll consider incorporating something like the above change into a future release in order to allow tablesaw to work as one would expect when the script is loaded after the DOMContentLoaded event fires. If it would help, I can create a pull request with those changes above. Thanks.

[porcus]

I've created a pull request to address this issue.

[DanielRuf]

Push. We still need this. Alternatively we can use patch-package.

[DanielRuf]

Hm, I guess interactive interactive is missing in the PR.

https://developer.mozilla.org/en-US/docs/Web/API/Document/readyState

This is for example the case when we use requireJS.

[porcus]

@DanielRuf - I'm sorry if this didn't work for your scenario. The interactive document readystate is not included in my PR, because based on my understanding and my tests, the document has an interactive readystate before the DOMContentLoaded event is triggered, so I can be reasonably certain that the DOMContentLoaded event has not fired by the time the document's readystate changes to interactive. In my case -- where I knew the document had not yet loaded by the time I initialized my tables -- the changes in my pull request were sufficient.

To be clear, the scenario not covered by the changes in PR #345 is the following:

• tablesaw is loaded after the DOMContentLoaded event
• an attempt is made to initialize tables using tablesaw while the document's readystate is still interactive (i.e. not yet completed/loaded)

I presume this^ describes your scenario. If you are unable to defer the initialization of tables until a later time, then perhaps you could test setting the domContentLoadedTriggered once the readystate transitions to interactive. I didn't write tests for that scenario, but feel free to take my pull request and build on it.

[DanielRuf]

Exactly. I could try to force always true but it seems the initialization is sometimes a bit flaky with requireJS (which is used in an ecommerce solution where we try to implement tablesaw). It's mainly meant as information that this might not always work in some cases like ours where we have lazyloaded js files.

[DanielRuf]

I'll try to dig a bit deeper then and provide some tests to cover this usecase.

[zachleat]

Merged! Thank you!