Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Add a tokenize worker to speed up code highlighting #928

Closed
kumar303 opened this issue Jul 8, 2019 · 7 comments
Closed

Add a tokenize worker to speed up code highlighting #928

kumar303 opened this issue Jul 8, 2019 · 7 comments

Comments

@kumar303
Copy link
Contributor

kumar303 commented Jul 8, 2019

Tokenizing a file to highlight the code makes the file render slowly. See #402 for STR -- a minified diff like this is the most dramatic example.

I confirmed in benchmarks that the tokenizer is the main offender so it's worth our time to try switching to a worker model.

Specifically, while profiling a production build from d07fbc3 and loading the minified diff from #402, tokenize() accounts for 95% of the time, i.e. 7.32 seconds.

@kumar303
Copy link
Contributor Author

kumar303 commented Jul 10, 2019

We need a Web Worker for this feature, which won't be supported until create-react-app 3.1 (roadmap). PR: facebook/create-react-app#5886 (it's supported now)

Before Web Workers are officially supported, we would need something like react-app-rewired or craco to set output.globalObject = 'this' in the webpack config, at least until webpack/webpack-dev-server#1595 is fixed. I have a basic PoC working but we could run into deployment issues.

@willdurand @bobsilverberg maybe we should try and wait until CRA 3.1 is released? What do you think?

@bobsilverberg
Copy link
Contributor

Sounds reasonable to me.

@willdurand
Copy link
Member

I think waiting for CRA 3.1 would be wise, even though the current PR does not support TypeScript. I guess we'll workaround that if needed, though.

I like this suggestion in the meantime: #402 (comment). I would even say that minified code is barely readable anyway, with or without syntax HL (I don't read minified code a lot but usually I find it more confusing when syntax HL is enabled).

@kumar303
Copy link
Contributor Author

Good call on disabling highlighting on large files. I'll do that in #942

@kumar303
Copy link
Contributor Author

This is supported in create-react-app out of the box now. This article explains how to do it with a workerize! import: https://kentcdodds.com/blog/speed-up-your-app-with-web-workers

@bobsilverberg bobsilverberg self-assigned this Jan 21, 2020
@bobsilverberg bobsilverberg added this to the 2020.01.30 milestone Jan 22, 2020
@bobsilverberg bobsilverberg removed this from the 2020.02.06 milestone Feb 5, 2020
@bobsilverberg
Copy link
Contributor

Removing this from the project as we determined this is not something we want to do at this time. See #1411 (comment)

@bobsilverberg bobsilverberg removed their assignment Sep 24, 2020
@stale
Copy link

stale bot commented Mar 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

@stale stale bot added the state:stale label Mar 26, 2021
@stale stale bot closed this as completed Jun 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants