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

An exception when trying it out the package in a chrome extension (service-worker.js file) #83

Open
medamine980 opened this issue Jul 31, 2023 · 10 comments
Labels
bug Something isn't working
Milestone

Comments

@medamine980
Copy link

Hey,
so I just tried out the code presented in the README.md file, and this error popped up
Uncaught (in promise) TypeError: (t.adapter || s.adapter) is not a function
I'd like to note that I am using webpack to bundle this file and also it works fine in the popup.

@vitonsky
Copy link
Contributor

@medamine980 hi, please provide more info to debug the problem.

  • what you do and for what?
  • what package you use? name and version
  • add example of code you use that are failed
  • add log or minimal reproducible code

@vitonsky vitonsky added the question Further information is requested label Jul 31, 2023
@medamine980
Copy link
Author

medamine980 commented Aug 1, 2023

  • Trying to use it in a browser extension, translate some text (a long one) from one language to another, the use of a background script is necessary since I can't use an injected script (content_script) because of cors and same origin policy.

  • Package name: @translate-tools/core, version: 0.2.16 (latest I guess)

  • The code that's causing the error:
    import { GoogleTranslator } from '@translate-tools/core/translators/GoogleTranslator';

    const translator = new GoogleTranslator();

    // Translate single string
    translator
    .translate('Hello world', 'en', 'fr')
    .then((translate) => console.log('Translate result', translate));

  • The error I get: Uncaught (in promise) TypeError: (t.adapter || s.adapter) is not a function, all I know is that this error happens only when I try the code above on the background page (I'm sure you are familiar with it since you worked on an extension before), but when I execute the same code in the popup script, everything works as expected.

@vitonsky
Copy link
Contributor

vitonsky commented Aug 1, 2023

@medamine980 did you tried another translators? If no, try this and add comments about status, is still the same error occurs for you or not. You can try YandexTranslator and LibreTranslateTranslator, also you can try GoogleTranslatorTokenFree.

Please, show a full error stack trace and give the source code of extension you work on

@medamine980
Copy link
Author

Okey I tried to use it on a new clean extension to make sure if the error is coming from the package, and in fact it is.
Here is the repo that I just deployed (the src folder contains the source code used in development and build is the one that should be loaded in the browser)

@medamine980 did you tried another translators? If no, try this and add comments about status, is still the same error occurs for you or not. You can try YandexTranslator and LibreTranslateTranslator, also you can try GoogleTranslatorTokenFree.

Please, show a full error stack trace and give the source code of extension you work on

@vitonsky
Copy link
Contributor

vitonsky commented Aug 1, 2023

@medamine980 as i know, manifest version v3 run code in service worker instead of background script.

It looks the problem are here. Translate tools did not tested for web workers and probably some API are not available in service worker environment. It looks you found a bug, translate tools must works well for every browser environment. Thanks for your report. It will fixed when we will detect whats code not available in service worker.

We have to investigate the stack traces to detect the problem. If you can do, i would appreciate your help. Otherwise i will investigate this bug myself when will have a time.

@vitonsky vitonsky added bug Something isn't working and removed question Further information is requested labels Aug 1, 2023
@medamine980
Copy link
Author

medamine980 commented Aug 2, 2023

So it seems, there is some variable adapter in a config object... idk if this is what you have actually written and just what webpack is giving but anyway, yeah the problem is in the service worker, more specifically the XMLHttpRequest, it seems that you're using it instead of fetch and sadly in manifest v3, XMLHttpRequest for the service worker is undefined... that's all I got for now.

@medamine980 as i know, manifest version v3 run code in service worker instead of background script.

It looks the problem are here. Translate tools did not tested for web workers and probably some API are not available in service worker environment. It looks you found a bug, translate tools must works well for every browser environment. Thanks for your report. It will fixed when we will detect whats code not available in service worker.

We have to investigate the stack traces to detect the problem. If you can do, i would appreciate your help. Otherwise i will investigate this bug myself when will have a time.

@vitonsky
Copy link
Contributor

vitonsky commented Aug 2, 2023

@medamine980 it is good findings. You are right, translators uses axios package for now for requests, that uses XHR under the hood.

I will investigate how to solve this problem better and probably will replace fetcher to a https://github.com/sindresorhus/ky

For now i working on refactoring this package, i think fix will be available on next week.

This bug shows me that we need a testing in real browser with tools like playwright/puppeteer, to ensure we cover all users scenarios

@medamine980
Copy link
Author

Alright, looking forward to the fix!

ky seems easier than fetch.

Can you help me understand how playwright/puppeteer would help you to test all scenarios? as far as I know those are tools used for web scraping, and I'm not sure other scenarios exist, I mean we have a normal script or a web worker script, and since it is compiled down (by webpack or other bundlers) to an old version of js, means most browser versions would work fine.

@medamine980 it is good findings. You are right, translators uses axios package for now for requests, that uses XHR under the hood.

I will investigate how to solve this problem better and probably will replace fetcher to a https://github.com/sindresorhus/ky

For now i working on refactoring this package, i think fix will be available on next week.

This bug shows me that we need a testing in real browser with tools like playwright/puppeteer, to ensure we cover all users scenarios

@vitonsky
Copy link
Contributor

vitonsky commented Aug 2, 2023

Can you help me understand how playwright/puppeteer would help you to test all scenarios?

Off course. We have to add tests for translators in a WebWorker context. WebWorker does not exists in nodejs as i know, so we can't implement test case properly. Thus we need an actual WebWorker environment. We can run tests in headless chrome with tools like playwright/puppeteer.

If you have idea how to test code in WebWorker context, with no run headless browser, i would be glad to know.

@medamine980
Copy link
Author

Well, there is something called Worker Threads in nodejs...

Otherwise you could somehow write the code you want to test in a script, and then write a nodejs script that will open the browser and execute your code in a webworker...
but that's kinda like what playwright/puppeteer does so no :')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants