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

Only bundle the Swiper modules that we use #4455

Merged
merged 1 commit into from
Dec 16, 2023

Conversation

absidue
Copy link
Member

@absidue absidue commented Dec 13, 2023

Only bundle the Swiper modules that we use

Pull Request Type

  • Build optimisation

Description

Currently we bundle the entirety of swiper. As swiper has a lot of features, that means we end up bundeling a lot of code that we don't need. This pull request switches to only bundling the swiper modules that we actually use. Previously the CSS was previously in the JavaScript bundle and Swiper injected it at runtime, now we have only the styles we need.

File Before After
renderer.js 2.01 MB (2115948 bytes) 1.94 MB (2041041 bytes)
web.js 1.66 MB (1744320 bytes) 1.59 MB (1669158 bytes)
swiper.css - 6.33 KB (6486 bytes)

Web Components have their own scope, known as a shadow DOM, so any CSS that you want inside the Web Component has to be inside that shadow DOM. Swiper supports injecting the CSS either by giving it links to CSS files or by giving it raw CSS. As we only want to inject the styles that Swiper needs, into the component, I decided to split it out into a separate CSS file. That created it's own challenges though, as we need to reference it from JavaScript it has to have a predictable name so the pattern of [name].[contenthash].css (renderer.abcdefg.css) that we use for the main CSS files wasn't going to work, because we don't know the hash in the JavaScript file. Originally I tried doing it with webpack's split chunks feature, but that resulted in the unpredictable name mentioned previously. In the end I decided to use the CopyWebpackPlugin with it's transformAll option to combine the 3 CSS files into one, the generated file still gets optimised, as the optimisations are done after copying.

Testing

Test that community posts with multiple images still work the same as before this pull request.

Example channel https://www.youtube.com/@MrBeast/community

You can only test on the channel page at moment, as the subscriptions page currently has a layout bug on the community tab, which also exists without this pull request.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.19.1

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 13, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 13, 2023 22:31
@FreeTubeBot FreeTubeBot merged commit 02ee4c8 into FreeTubeApp:development Dec 16, 2023
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 16, 2023
@absidue absidue deleted the swiper-tree-shaking branch December 16, 2023 13:51
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Dec 20, 2023
* development:
  Bump the babel group with 2 updates (FreeTubeApp#4466)
  Bump the eslint group with 4 updates (FreeTubeApp#4468)
  Bump @seald-io/nedb from 4.0.2 to 4.0.3 (FreeTubeApp#4469)
  Bump marked from 11.0.1 to 11.1.0 (FreeTubeApp#4470)
  Bump github/codeql-action from 2 to 3 (FreeTubeApp#4472)
  Only bundle the Swiper modules that we use (FreeTubeApp#4455)
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.

5 participants