JqueryPlugins, HTTPStatusCodes and KeyCodes rolled into uv#1500
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Thanks, @Geoffsc, this all looks good to me!
I did a little bit of searching, and I found one lingering reference to @edsilv/jquery-plugins that I'm not sure what to do with:
Line 17 in 72013bd
I don't know much about the various methods of Javascript packaging, so I'm not sure under what circumstances this build script is used (or its resulting artifacts are required). Since the library is no longer in our package.json, maybe all we need to do is remove the reference from the build script. But I don't know what it would take to actually test that everything is okay with that particular build of the package. Anyone know more about this than I do?
On a semi-related note, I also notice that there's this mobile.html page which has hard-coded unpkg.com links to some packages, including an ancient version of jQuery. Does anyone know if that's used for anything? It looks like @edsilv created it in 2020 for some mobile testing, and I suspect it's no longer essential, but we probably shouldn't delete it without being a little more confident. :-)
(I should note that the mobile.html thing has nothing directly to do with this PR since none of the packages included are the ones removed here... but just mentioning it before I forget; it will become more important when we get to @edsilv/utils).
e512b28 to
e5a49e8
Compare
|
Thanks @demiankatz I went ahead and added a commit to remove that reference from But we should definitely test it :) |
That's the bit I'm somewhat unsure about. It looks like the examples page is running from the UMD bundle, which gets injected into the default examples page index.html as part of the webpack configuration. I think if we wanted to test the ESM bundle, we would have to import the UV class from the compiled module instead -- but I'm not sure exactly how that would work. It's possible that the jQuery stuff simply won't work right in this mode, which may be why it's previously been pulled in from external CDNs... but again, my Javascript packaging knowledge is somewhat limited, so I don't know much about all the possible nuance. |
|
I have tested this both in desktop and in mobile view. Everything works as expected in desktop view. I believe the bug found for mobile view is not related to this PR. I am confident that @demiankatz is more qualified to approve this PR. Thanks @Geoffsc for the work you have done here :-) |
|
This is definitely fine for the UMD build, but I think we need to figure out a way to test the ESM build, since I'm concerned that it may be broken. However, I don't know how to do that, or whether anyone relies on it. If nobody needs it, maybe we can simplify by dropping support -- but I suspect we may want it in the future. |
|
ESBuild fails for me due to a LESS issue where it thinks the CSS rgb function is a LESS function, so that's probably been the case for a while. Might be an easy fix if the less loader is updated? As far as I can tell for the There's |
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @LlGC-jop. Since the ESBuild issues do not seem to be directly related to the changes in this PR, I'll open an issue to track the need to investigate/test, and we can merge this as-is.
JqueryPlugins, HTTPStatusCodes and KeyCodes rolled into uv
#1380