Skip to content

Conversation

matthargett
Copy link
Contributor

What?

This fixes a number of lagging dependencies that were producing a lot of console (terminal and browser) noise and several security vulnerabilities.

Why?

I was trying to fix the WebXR issues I've enountered in Hubs over the last couple of meetups, but found myself stymied by the dozens and dozens of warnings during initial setup and that several storybook entries were rendering blank pages. I decided it would be a better service to get this repo generally easier to run in fresh checkouts on modern operating system and node versions. I hope this helps broaden contributions in the future beyond myself.

How to test

  1. npm ci should now complete with a lot less noise on the console about deprecations and security vulnerabilities.
  2. npm test should now pass more consistently without having to install nodejs 12.
  3. npm storybook should now allow you to click and see every single component render, without any blank pages.
  4. npm local now has significantly less noise, making it easier to see warnings and errors
  5. npm bundle-analyzer will show a nice reduction in the bundle size thanks to optimizing for Chrome 91 (Oculus Go) on the low-end instead of Chrome 51 and IE11. (see comments in the babel conf for compatibility matrix choices)

Documentation of functionality

Everything should work as before, just better and with less noise for new (and existing) developers).

Limitations

Getting an e2e QA setup was beyond my abilities and time, so I'd appreciate some help testing this more deeply in a multiuser way. I've expanded unit testing a bit to cover the areas I was concerned about breaking (PDFs), and am happy to add more unit tests for whatever surfaces during an e2e test so that all future modifications can get a better signal on regressions.

Alternatives considered

I tried to get meaningful reduction in noise, rather than updating to absolute latest and greatest package versions. In most cases, bumping one package required bumping another (especially corejs and babel to support new kinds of exports). If this goes well, we can do another pass. If that goes well, we can look at gently cherry-picking more things from upstream aframe and threejs into the Hubs fork.

Open questions

I need help with end-to-end QA, and am happy to do a 1-hour pair-testing session to work through any issues.

Additional details or related context

If this works well, and there's no immediate requests, I'll move onto solving my WebXR issue on Apple Vision Pro.

…pment loop. Fix numerous storybook rendering errors, fortifying use of prop-types were possible so that better feedback instead of a blank page. Rebase minimum browser expectations for 2025, supporting final browsers available on Oculus Go (Chrome 91), 2012 MacBook (Safari 15), iPhone 7 (Safari), Windows 7 (Edge/Chromium 109), and HTC Vive Wolvic (Gecko 116) as the basis.
…atest minor/patch and adding some tests. Bump a few other packages to eliminate more warning noise.
… tentacles are little too much to do in this PR. that said, I feel like the functionality is verified enough at the unit level that it won't be a waste of time to ask for a QA pass.
@Exairnous
Copy link
Contributor

@matthargett I tried building this with the Custom Docker Build Push GitHub Action, but it failed to build. It looks like the Dockerfile hasn't been updated to use Node 22 (however, I did a quick test of that and updating it seemed to cause issues with the installation of PhantomJS, probably because the new Node image uses a newer Debian release, although I only tried the 22 Docker image, so maybe one of the other variations wouldn't have that problem). Can you confirm?

@matthargett
Copy link
Contributor Author

building this with the Custom Docker Build Push GitHub Action, but it failed to build. It looks like the Dockerfile hasn't been updated to use Node 22 (however, I did a quick test of that and updating it seemed to cause issues with the installation of PhantomJS, probably because the new Node image uses a newer Debian release, although I only tried the 22 Docker image, so maybe one of the other variations wouldn't have that problem).

Great feedback! It looks like we can:

  1. use node:20-buster image, which uses nodejs 20 with the older Debian where phantomjs still works.
    or
  2. use the newer debian image and the last update to the Ubuntu package got PhantomJS: https://launchpad.net/ubuntu/+source/phantomjs/2.1.1+dfsg-2ubuntu1

option 1 seems like the shortest throw, and I can submit a PR to gently migrate to the new docker image so we can see how it works in deployment. it's not clear to me where to submit that PR, though.

also note: phantomjs has been archived and abandoned for over 5 years now (I was surprised to see SlimerJS is also shut down.) moving forward, it seems like moving to puppeteer / headless-chrome might be better for long-term viability. that said, the options above paint a way forward in the short term without changing the headless browser tech.

cheers!

@Exairnous
Copy link
Contributor

@matthargett Thanks!

I'm thinking that option 1 sounds like it has more of a chance to preserve the arm64 support, so potentially that's the better option (@ajlennon ?). As to where to submit the PR, I'm not quite sure what you mean. As far as I know, the spot where this would be modified is the RetPageOriginDockerfile in this repository, so I think you can just update this PR.

also note: phantomjs has been archived and abandoned for over 5 years now

Yes. We don't use it directly (it looks like a dependency of a dependency), so it would be great to be able to remove it, but that seems like it could be quite tricky (I haven't looked into it too much, though).

Exairnous and others added 19 commits June 16, 2025 13:04
What: Prevents Discord messages from different users that are sent within a minute of each other from being grouped together under the first user that sends a message by also checking the name given for the sender instead of just the sender's id.  This mostly ensures that all chat messages are attributed to the correct user.

Why: Users in Hubs can have the same name so the chat messages are grouped by a sender id, but because all the messages from Discord come through the Discord bot, the sender id is the same for all of them regardless of who sent them in Discord.

Note: this will still group messages improperly if the Discord users have the same display name in the Discord server but preventing that would likely require much more extensive and broad reaching changes (probably in reticulum and the Hubs Discord bot repository as well) and it would do little to reduce confusion.
…houldn't be installed anyway since its a devDependency, but let's handle that as a separate optimization
…e (antipattern) of git dependencies pulling in their devDependencies with preinstall checks.
…cript shenanigans to execute phantomjs, and we want to try only upgrading one aspect at a time
…pment loop. Fix numerous storybook rendering errors, fortifying use of prop-types were possible so that better feedback instead of a blank page. Rebase minimum browser expectations for 2025, supporting final browsers available on Oculus Go (Chrome 91), 2012 MacBook (Safari 15), iPhone 7 (Safari), Windows 7 (Edge/Chromium 109), and HTC Vive Wolvic (Gecko 116) as the basis.
…2. Fix a conflicting dependency in the admin, which required a pretty big jump to react-admin but also fixed a remote code exectuion vulnerability.
…tical vulnerabilities. update the webpack config to generate proper source maps to ease debugging.
… import warnings. longer term, we want to bump pdfjs to latest but I'm trying to avoid a full transition to ES modules (from CommonJS) in this PR
…will fail unless we manually supply types for pdfjs until we upgrade to latest pdfjs.
…bpack configs. add flag to build commands so child errors are surfaced instead of suppressed.
…e at the globalThis and other things from a different angle to see if that works better.
…avoiding code injection vulnerabilities). Add the specific hash for the Google Analytics inline JS, and a whitelist for the jsdelivr CDN used for fonts.
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.

2 participants