-
Notifications
You must be signed in to change notification settings - Fork 82
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
Run project config parse inside web worker #794
base: master
Are you sure you want to change the base?
Run project config parse inside web worker #794
Conversation
@@ -197,6 +200,41 @@ export class ProjectConfigManager { | |||
return error; | |||
} | |||
|
|||
private async handleDatafileInWebWorker(newDatafile: string | object): Promise<OnReadyResult> { | |||
const workerCode = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be handled by some loader inside rollup; like this one: https://www.npmjs.com/package/rollup-plugin-web-worker-loader
@PepijnSenders - thanks for submitting the PR! Looked through it and looks pretty slick. As this is technically a feature request, we'll want to document it through our feature request pipeline as well. Please submit a request there when you have the chance - thanks in advance! |
Hey @opti-jnguyen, I was already discussing this change with one of your Solution Architects, do you need both? Happy to do so! |
Hi @PepijnSenders , I apologize for the delayed reply here. It's my understanding that you're also speaking with one of our Solution Architects. If he has not mentioned this already, we would greatly appreciate if you could please submit this through the feature request pipeline as well. Rest assured we are tracking the issue within our Development team. If it's not too much trouble to also submit this via the feature request pipeline, this will allow for more visibility across our Product and Engineering org. Lastly, I believe Joshua has spoken with you about next steps for evaluating this change in your environment. I cannot comment on when we can look at this in more detail past our initial review unfortunately due to prior planned work. Thanks for your collaboration and if you have any questions please let us know. |
This was last mentioned in internal spike/research ticket FSSDK-8510. I've requested a dev ticket be created. |
Oops. The development ticket for this is FSSDK-9614. We should be pointing it shortly. |
Any update? |
I bumped the internal work ticket for comment. |
Summary
The
createProjectConfig()
function is quite slow and runs sync with the application if the datafile is provided as anobject
.This function could easily be run inside a web worker, freeing up the main thread, and we could use the same promise to track when it's done as we use for when we fetch the datafile.
For this PR to work, we'd still need:
createInstance
function.