fix: Avoids converting a gif into a function #247
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| scriptContent = await rsp.text(); | ||
| env.$currentScriptId$ = instanceId; | ||
| run(env, scriptContent, scriptOrgSrc || scriptSrc); | ||
| } | ||
| runStateLoadHandlers(instance!, StateProp.loadHandlers); |
There was a problem hiding this comment.
Not entirely sure if this state change should be called or not
|
Open for ideas about how to test that since the expected behaviour is that a gif is requested with no other side effects. Is there a way to verify that using playwright? 😅 Also, the integration that made us find that error was Google Adsense but that should happen everytime a script inside partytown requests something other than other script. |
leocwolter
changed the title
leocwolter
changed the title
|
This looks like a good fix, but what you also be able to add some tests to this to ensure it continues to work, and isn't breaking other tests? |
src/lib/web-worker/worker-exec.ts
Outdated
| @@ -27,7 +27,8 @@ export const initNextScriptsInWebWorker = async (initScript: InitializeScriptDat | |||
| let errorMsg = ''; | |||
| let env = environments[winId]; | |||
| let rsp: Response; | |||
|
|
|||
| let contentTypeBlocklist = ["image/gif"] | |||
There was a problem hiding this comment.
I think it's a good idea to validate that's javascript before we execute it. But could we do the opposite here, where we have a whitelist of content types which are valid javascript?
There was a problem hiding this comment.
My concern with using a whitelist was that maybe some request doesnt return the correct contentType even though they are actually javascripts. If that is not something that you guys think could happen I can implement it as a white list :)
src/lib/web-worker/worker-exec.ts
Outdated
| env.$currentScriptId$ = instanceId; | ||
| run(env, scriptContent, scriptOrgSrc || scriptSrc); | ||
| let responseContentType = rsp.headers.get("content-type") | ||
| if (!contentTypeBlocklist.some(ct => ct === responseContentType)){ |
There was a problem hiding this comment.
This is a good idea, but rather should go with a whitelist instead of blacklist.
|
Finished the fixes and added end to end test @adamdbradley |
|
This is great, thanks so much for fixing this up! |
Summary
Fixes an issue where partytown prints an error when a third party scripts requests a resource different from a script.
Closes #245