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

Add interactive check to ready event handler #1659

Closed
wants to merge 11 commits into from

Conversation

dsyer
Copy link
Contributor

@dsyer dsyer commented Aug 2, 2023

If HTMX is imported in a module the readyState is "interactive" so the extension processing happens too late. This would fix it.

See #1655

dependabot bot and others added 10 commits July 29, 2023 17:01
…1.0 in /www/static/test/1.9.4/test/servers/ws (bigskysoftware#1632)

Bump golang.org/x/crypto in /www/static/test/1.9.4/test/servers/ws

Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.0.0-20210322153248-0c34fe9e7dc2 to 0.1.0.
- [Commits](https://github.com/golang/crypto/commits/v0.1.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… in /www/static/test/1.9.4/test/servers/ws (bigskysoftware#1633)

Bump golang.org/x/net in /www/static/test/1.9.4/test/servers/ws

Bumps [golang.org/x/net](https://github.com/golang/net) from 0.0.0-20210405180319-a5a99cb37ef4 to 0.7.0.
- [Commits](https://github.com/golang/net/commits/v0.7.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… in /www/static/test/1.9.4/test/realtime (bigskysoftware#1642)

Bump golang.org/x/net in /www/static/test/1.9.4/test/realtime

Bumps [golang.org/x/net](https://github.com/golang/net) from 0.0.0-20211015210444-4f30a5c0130f to 0.7.0.
- [Commits](https://github.com/golang/net/commits/v0.7.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…1.0 in /www/static/test/1.9.4/test/realtime (bigskysoftware#1643)

Bump golang.org/x/crypto in /www/static/test/1.9.4/test/realtime

Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.0.0-20210817164053-32db794688a5 to 0.1.0.
- [Commits](https://github.com/golang/crypto/commits/v0.1.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…est/1.9.4/test/servers/ws (bigskysoftware#1646)

Bump github.com/labstack/echo/v4

Bumps [github.com/labstack/echo/v4](https://github.com/labstack/echo) from 4.3.0 to 4.9.0.
- [Release notes](https://github.com/labstack/echo/releases)
- [Changelog](https://github.com/labstack/echo/blob/master/CHANGELOG.md)
- [Commits](labstack/echo@v4.3.0...v4.9.0)

---
updated-dependencies:
- dependency-name: github.com/labstack/echo/v4
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Update podcasts.csv

fix spelling of 21st
fix case of devMode.fm to match show notes

* Update podcasts.csv

Add podcasts not found on LinkedIn

* Update podcasts.csv

Add latest appearance

* Update podcasts.csv

Add Hype for Hyperscript

* Update podcasts.csv

grug make successful rick roll

* Update podcasts.csv

fix timestamp for GitHub chopping off beginning of video

* Update podcasts.csv

Unfiltered
airport shenanigans
Update the static site generator to include the date and author
automatically as part of the essays. I made a couple changes to the
underlying static structure to make this a little more seamless,
including:

* New "essay.html" template for posts in `/essays`
* Added an author taxonomy (can eventually be author pages)
* Added Carson as the author of all the existing esssays
* Cleaned up manual date entires, where they existed

All this will make it easier to publish guest essays and sort the essays
(chronologically, by topic, or by author) as the essay base expands.
Make the docs a little bit shorter so that the old form is still
documented without repeating information (and the new form is
emphasized). I also fixed a misplaced double quote in one of the
examples.
@alexpetros
Copy link
Collaborator

This seems good but could you explain the problem and your solution a little more thoroughly?

@dsyer
Copy link
Contributor Author

dsyer commented Aug 2, 2023

There's a sample here: https://github.com/scratches/htmx-demo. If you prefer Python there's a simpler version in the "django" branch. They both work as they are, but they use a hacked version of htmx.js with the change in this PR. If you switch to the regular HTMX (e.g. from unpkg.com) it will break because the extension isn't installed before the ready listener fires.

You can actually get the same result if you replace the module stuff with normal <script> tags but add the "async" attribute. Module imports are asynchronous, so I guess it comes to the same thing. E.g:

	<script async src="https://unpkg.com/htmx.org"></script>
	<script async src="https://unpkg.com/htmx.org/dist/ext/sse.js"></script>

@alexpetros
Copy link
Collaborator

alexpetros commented Aug 2, 2023

Got it, thank you. You're right, and the async issue has come up before (#1485). I also think it makes sense to initialize htmx last (after the extensions) anyway.

Just to make sure I understand fully, since there are only three readyStates, this is logically equivalent to following?

if (getDocument().readyState === 'complete')

@alexpetros alexpetros added enhancement New feature or request ready for review Issues that are ready to be considered for merging labels Aug 2, 2023
@dsyer
Copy link
Contributor Author

dsyer commented Aug 3, 2023

I guess it must be. You want to make that change? I tried it in the sample and it worked for me.

Reading that other issue though, I can see this might not work in all circumstances. It's simple though and it works for me, and seems unlikely to break any existing apps (I'm no kind of expert, just trying to find a solution).

@guilleiguaran
Copy link

I've tested this with Vite/Rollup (as in #1469) and it's working as expected.

If HTMX is imported in a module the readyState is "interactive"
so the extension processing happens too late. This would fix it.
@alexpetros
Copy link
Collaborator

Thank you both for verifying, this looks great. I'm not entirely sure why the CI is failing, because it only does so intermittently on my local machine.

@1cg is it possible this timeout just needs to be longer?

htmx/test/core/api.js

Lines 18 to 28 in 66387c0

setTimeout(function() {
try {
var div = make("<div id='d1' hx-get='/test' hx-swap='outerHTML'></div>");
div.click();
server.respond();
byId("d1").getAttribute("foo").should.equal("bar");
done();
} finally {
htmx.off("htmx:load", helper);
}
}, 10)

@1cg
Copy link
Contributor

1cg commented Aug 4, 2023

yeah, I'm going to merge this and I'll take a look at that test today

@1cg
Copy link
Contributor

1cg commented Aug 4, 2023

actually, @dsyer can you make this PR against dev for me?

thank you!

@dsyer
Copy link
Contributor Author

dsyer commented Aug 4, 2023

I don't think I know how to do that. I could close this one and open a new PR? Or you could walk me through it if it's possible otherwise and I'm just too stupid.

1cg#1 also #1668 because I don't know what "dev" means.

@1cg
Copy link
Contributor

1cg commented Aug 4, 2023

lmao i have no idea how to do it either I just ask other people to do it

@dsyer
Copy link
Contributor Author

dsyer commented Aug 4, 2023

Sorry, I didn’t check if I needed to rebase before I made those PRs. I’m AFK now but I can do it tomorrow.

@alexpetros
Copy link
Collaborator

actually, @dsyer can you make this PR against dev for me?

thank you!

@dsyer what he means is that this PR is set to merge into the "master" branch and there's a setting (should be at the top, when you edit the PR) to chose to merge it against the "dev" branch instead (that's the one we use for commits that will be in the next release).

@dsyer
Copy link
Contributor Author

dsyer commented Aug 4, 2023

I know what he meant, but I don’t think there’s a way to change the base branch once the PR is submitted.

@alexpetros alexpetros changed the base branch from master to dev August 4, 2023 22:40
@alexpetros
Copy link
Collaborator

Tried to change the branch but unfortunately the PR has a bunch of commits from master in it now, so I cherry-picked your commit and put it in PR #1672 (should still say you authored it).

@alexpetros alexpetros closed this Aug 4, 2023
@alexpetros
Copy link
Collaborator

You're in: 347ce7a

Sorry about the weirdness

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants