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

Feature - optimise build size and performance with PrismLight syntax highlighter and add a new highlighter style selector #97

Closed

Conversation

barrymun
Copy link
Contributor

@barrymun barrymun commented Jan 1, 2025

Description

  • Optimise build size and performance with PrismLight syntax highlighter. PrismLight cannot be used as it does not support a lot of the languages that are selectable in the dropdown.
  • Add a new highlighter style selector.
  • Consolidate the usage of theme and toggleTheme so that duplicated logic is removed.
  • Consolidate selector component logic into a new Selector component.
  • Language now persisted in localStorage ("languageName").
  • Highlighter style now persisted in localStorage ("highlighterStyleName").

Type of Change

  • ✨ New snippet
  • 🛠 Improvement to an existing snippet
  • 🐞 Bug fix
  • 📖 Documentation update
  • 🔧 Other (please describe): Performance optimisations and adding a new selector.

Checklist

  • I have tested my code and verified it works as expected.
  • My code follows the style and contribution guidelines of this project.
  • Comments are added where necessary for clarity.
  • Documentation has been updated (if applicable).
  • There are no new warnings or errors from my changes.

Related Issues

Closes #94

Additional Context

Screenshots (Optional)

Click to view screenshots

new build size:
Screenshot 2025-01-01 at 17 14 25

new localStorage keys:
Screenshot 2025-01-01 at 18 50 06

Copy link

netlify bot commented Jan 1, 2025

Deploy Preview for quicksnip ready!

Name Link
🔨 Latest commit 5bbd91b
🔍 Latest deploy log https://app.netlify.com/sites/quicksnip/deploys/6776930dc9d70a00078e2857
😎 Deploy Preview https://deploy-preview-97--quicksnip.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@barrymun barrymun changed the title Feature - optimise build size and performance with PrismaLight syntax highlighter and add a new highlighter style selector Feature - optimise build size and performance with PrismLight syntax highlighter and add a new highlighter style selector Jan 1, 2025
@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

@psychlone77 @saminjay This PR handles (and tests) the usage of PrismLight to reduce the build size, and from what I can see while testing this appears to function exactly as before.

A new highlighter style selector has been added, but this is only proof of concept for now and is therefore subject to change. I've also done some consolidation to remove duplicated code around theme selection ("dark" or "light"), and selector component logic.

Finally, the selected highlighter theme is saved in localStorage, as well as the language (but the latter of these two changes may not be desirable), so that these selections persist for the user in the same manner as the theme.

The feature discussion for these changes is here: #94, and everything is subject to change if these alterations are not welcome. Thanks.

@Mathys-Gasnier
Copy link
Collaborator

It doesn't seem to highlight certain language like C or CPP

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

It doesn't seem to highlight certain language like C or CPP

I just checked and it was working for me on 2 randomly selected styles for c and cpp. Was there a particular style that didn't work for you?

"coyWithoutShadows" style:
Screenshot 2025-01-02 at 12 19 35

"pojoaque" style:
Screenshot 2025-01-02 at 12 20 04

@Mathys-Gasnier
Copy link
Collaborator

image

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

image

What style had been selected in the highlight style selector?

@Mathys-Gasnier
Copy link
Collaborator

oneDark

@Mathys-Gasnier
Copy link
Collaborator

But i checked a couple more, and none color the text

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

That is very strange. All of the selections seem to be working for me. This is with oneDark selected:

Screenshot 2025-01-02 at 12 39 03

I will look into this further, but can you confirm you've pulled the latest code and try removing your node_modules and re-installing with npm ci?

@Mathys-Gasnier
Copy link
Collaborator

I'm using the preview that's available here on your PR
image
It deploys your latest commit as a preview

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

I'm using the preview that's available here on your PR image It deploys your latest commit as a preview

Apologies, I thought you were testing locally. I've also tried on the deploy and confirmed it is also working there for me:

Screenshot 2025-01-02 at 13 01 33

Ddi you try clearing browser cache?

@barrymun barrymun closed this Jan 2, 2025
@barrymun barrymun reopened this Jan 2, 2025
@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

Didn't mean to close. Re-opened.

@Mathys-Gasnier
Copy link
Collaborator

I've tested it one brave, chrome, firefox, all with and without extension and it didn't work on a signe one of them... I think there is something wrong with this...

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

Here is another test with a different highlighter style on the deploy:

Screenshot 2025-01-02 at 13 03 25

@Mathys-Gasnier
Copy link
Collaborator

But on the screen you are providing it's not working...
currently on prod we have this:
image
Not just a background + text color, but proper syntax highlight

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

But on the screen you are providing it's not working... currently on prod we have this: image Not just a background + text color, but proper syntax highlight

Ok I understand what you mean now. I will look into this.

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

@Mathys-Gasnier I think i've fixed it. It seems that PrismLight is not suitable for our use case as a lot of the languages are discounted. This was hidden on my local as it was cached. However, this does mean that the build size cannot be reduced:

Screenshot 2025-01-02 at 13 24 41

@Mathys-Gasnier
Copy link
Collaborator

Sad, but i think that size is for good, since it allows for snippets to be read easier.

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

Unfortunately it seems that way. Although the original premise of this PR is now redundant, I am interested to see if you and the other maintainers (+ the community) would find the other changes here useful.

@Mathys-Gasnier
Copy link
Collaborator

The theme switch is cool, but i don't really think it's necessary, and for the Select, I think another PR was created that migrates that to Shadcn and tailwind

@barrymun
Copy link
Contributor Author

barrymun commented Jan 2, 2025

The Select component code remains the same (apart from some minor changes that handle text overflow with ellipsis), but it was just extracted into its own component. In that case I can close this PR if you are happy for me to do so?

@dostonnabotov
Copy link
Owner

@barrymun, regarding the theme switch, we might come back to it in the future. But right now, I also agree with @Mathys-Gasnier. It's better to keep things simple.

@barrymun barrymun closed this Jan 2, 2025
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.

[Feature] - Use PrismLight syntax highlighting instead of Prism to improve performance and reduce build size
3 participants