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

feat: add configuration to run specific scripts on main thread #200

Conversation

DanBeckDev
Copy link
Contributor

@DanBeckDev DanBeckDev commented Jun 21, 2022

Feature as requested by: #161

It was originally suggested that the configuration should look like this:

<script>
partytown = {
  loadWithPartytown: (url) => {
     if (/* your own logic for what to load in partytown vs not */) return true;
     return false;
  }
}
</script>

I have created it as an array:

/**
   * This array can be used to filter which script are executed via
   * Partytown and which you would like to execute on the main thread.
   * 
   * @example loadScriptsOnMainThread:['https://test.com/analytics.js']
   * // Loads the `https://test.com/analytics.js` script on the main thread 
*/
<script>
partytown = {
  loadScriptsOnMainThread: ['https://test.com/analytics.js']
}
</script>

This is because when the function gets serialized the function is a string and cannot execute.
If we wish to proceed with the idea above I am happy to extend the serialisation functionality but I deem it overkill myself.

@vercel
Copy link

vercel bot commented Jun 21, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
partytown ✅ Ready (Inspect) Visit Preview Jun 24, 2022 at 3:19PM (UTC)

src/lib/types.ts Outdated Show resolved Hide resolved
src/lib/web-worker/worker-script.ts Outdated Show resolved Hide resolved
@DanBeckDev
Copy link
Contributor Author

Thank you for the review @khuezy.
Please see my latest commit resolving the issues found above.

Copy link

@khuezy khuezy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I'm not a code owner so I don't think my approval would be sufficient.

@DanBeckDev
Copy link
Contributor Author

@steve8708 Any chance I could get a review, please?

@steve8708
Copy link
Contributor

Absolutely, @adamdbradley is crunching on a major release for Qwik City and then can hop in here for a review

@robertwatts
Copy link

robertwatts commented Jul 19, 2022

Hello, thank you so much for taking this on @DanBeckDev. We want to use PartyTown on wonderbly.com but are blocked by this issue. If @steve8708 or @adamdbradley is able to take a look at this feature so it can be released we would be very grateful - I'm sure this would be useful for many others as well.

@garethweaver
Copy link

This is great, thanks!

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.

None yet

6 participants