istio.io
istio.io copied to clipboard
istio
serve Splide and Three.js locally instead of via CDN
Description
Serve Splide and Three.js locally via static directory to replace external CDN usage.
Fixes #9723
Command run to download the css and js file -
curl -L -o static/js/three.min.js https://cdnjs.cloudflare.com/ajax/libs/three.js/r68/three.min.js
curl -L -o static/js/splide.min.js https://cdn.jsdelivr.net/npm/@splidejs/splide@latest/dist/js/splide.min.js && curl -L -o static/css/splide.min.css https://cdn.jsdelivr.net/npm/@splidejs/splide@latest/dist/css/splide.min.css
Reviewers
- [ ] Ambient
- [ ] Docs
- [ ] Installation
- [ ] Networking
- [ ] Performance and Scalability
- [ ] Extensions and Telemetry
- [ ] Security
- [ ] Test and Release
- [ ] User Experience
- [x] Developer Infrastructure
- [ ] Localization/Translation
Hi @AritraDey-Dev. Thanks for your PR.
I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
It's been a few years since I raised the request. Does this actually make sense?
Pros of CDN: loads quicker in parallel? Cons: external dependency, versions can change upstream and break site (unless we pin them?)
Yes, definitely makes sense. The concern you mentioned—external changes breaking the site—is actually what's causing issues when the scripts are loaded from the static folder. That’s why I’ve marked this PR as a draft for now. There are a few UI breaking changes I’m still working through.
Do you recommend I continue refining the PR and eventually open it for review?
Are these files going to be bundled/minified? Should they be?
(And if so, should we hold off on this until the esbuild PR has gone through?)
These files are already minified,look at the extension min.js, min.css
I believe they should definitely be minified, as that reduces size and boosts performance.
(And if so, should we hold off on this until the esbuild PR has gone through?)
I believe we don't need to hold off until the esbuild PR goes through because these Splide files are used directly by HTML files and the esbuild PR is focused on changing js files, so they are independent changes.
It's still JavaScript that we serve, and if we are going to have a single minified JS file which has its tree shaken, then we should have it only serve the code used by splide.
Should we have some sort of npm pulling splide situation like we have for the others? By making this change we move from latest to pinning the version at this exact point in time.
(That's not to say it's not worth doing, but we should look to do it properly if we are making any change.)
makes sense! I think we can add the particular versions of Splide (4.1.4) and Three.js (0.162.0) to the netlify_install target, then in gen_site.sh we can copy over those files if available from node_modules/ or fall back to existing static JS files,
and if there's a change we can figure that out easily through version pinning
I don't see Three.js here? And I still think these should be included in our esbuild bundle?
I was thinking of adding Three.js via Docker tools and pinning the version that way. Would that be a better approach instead of adding three.module.js directly to the repository? This way, updating the Three.js version would be easier. If we include three.module.js in the repo, it could become difficult to update it every time.
@craigbox I've added three.js in netlify_install. If this approach looks better, I will go ahead and add it to the tools Dockerfile. Please let me know.
@AritraDey-Dev: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| doc.test.multicluster_istio.io | 22a8f518b419e584fa18bde1589b8e8e69368462 | link | true | /test doc.test.multicluster |
| doc.test.profile-minimal_istio.io | 22a8f518b419e584fa18bde1589b8e8e69368462 | link | true | /test doc.test.profile-minimal |
| doc.test.profile-none_istio.io | 22a8f518b419e584fa18bde1589b8e8e69368462 | link | true | /test doc.test.profile-none |
| doc.test.profile-default_istio.io | 22a8f518b419e584fa18bde1589b8e8e69368462 | link | true | /test doc.test.profile-default |
| doc.test.dualstack_istio.io | 22a8f518b419e584fa18bde1589b8e8e69368462 | link | true | /test doc.test.dualstack |
| doc.test.profile-demo_istio.io | 22a8f518b419e584fa18bde1589b8e8e69368462 | link | true | /test doc.test.profile-demo |
| doc.test.profile-ambient_istio.io | 22a8f518b419e584fa18bde1589b8e8e69368462 | link | true | /test doc.test.profile-ambient |
| lint_istio.io | 722b28df945eecc59235c4233910590c2b8877a6 | link | true | /test lint |
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.