analytics-debugger / ga4mp

GA4MP
MIT License
40 stars 12 forks source link

Incompatible with webpack #20

Closed znorman-harris closed 1 year ago

znorman-harris commented 1 year ago

Hey There!

Great library and much easier than working directly with the REST API. I have discovered a problem when to bundle it with my app using webpack.

There's two things that happen:

  1. I get a warning when building my node.js app about the require statement within sendRequest:

WARNING in ../../node_modules/@analytics-debugger/ga4mp/dist/ga4mp.umd.js 187:16-31 Critical dependency: the request of a dependency is an expression

This is the exact line where there is a problem:

image

  1. Because of that issue with the import, at runtime webpack throws an error saying "Unable to find module 'http'"

If I change your require statement to be "require('https')" then I can build and run without a problem.

Any way you could statically import http and https at the top of the files so that webpack can fully resolve the dependencies?

Thanks!

thyngster commented 1 year ago

Ops lame me. I'll rewrite that this week.

On Thu, Jun 22, 2023, 00:44 Zachary Norman @.***> wrote:

Hey There!

Great library and much easier than working directly with the REST API. I have discovered a problem when to bundle it with my app using webpack.

There's two things that happen:

  1. I get a warning when building my node.js app about the require statement within sendRequest:

WARNING in ..@.***/ga4mp/dist/ga4mp.umd.js 187:16-31 Critical dependency: the request of a dependency is an expression

This is the exact line where there is a problem:

[image: image] https://user-images.githubusercontent.com/31664668/247779167-a5bde164-4148-4363-898d-93e0e193b2f8.png

  1. Because of that issue with the import, at runtime webpack throws an error saying "Unable to find module 'http'"

If I change your require statement to be "require('https')" then I can build and run without a problem.

Any way you could statically import http and https at the top of the files so that webpack can fully resolve the dependencies?

Thanks!

— Reply to this email directly, view it on GitHub https://github.com/analytics-debugger/ga4mp/issues/20, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALM4JC25YEETJ7L2MGDLRTXMN2NDANCNFSM6AAAAAAZPMKWLM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

znorman-harris commented 1 year ago

Awesome, thanks for the quick response!

thyngster commented 1 year ago

This should had been addresed on last version 0.0.8 . lmk

znorman-harris commented 1 year ago

Sorry if I wasn't clear before, but its still an issue.

Webpack can't resolve the HTTP or HTTPS lib dynamically from a variable name at runtime and is still giving me an error:

rejected promise not handled within 1 second: Error: Cannot find module 'https'
extensionHostProcess.js:105
stack trace: Error: Cannot find module 'https'
    at webpackContextResolve (c:\Users\      \dist\apps\client\main.js:17839:11)
    at webpackContext (c:\Users\      \dist\apps\client\main.js:17834:11)
    at sendRequest (c:\Users\      \dist\apps\client\main.js:17504:41)
    at c:\Users\      \dist\apps\client\main.js:17778:13
extensionHostProcess.js:105

To be more clear, it needs an explicit import like this to work:

image

I'm not an expert at webpack, so there might be some other kind of config I need, but when I use the explicit import I'm able to send events and have it work just fine.

thyngster commented 1 year ago

I updates it based on recommendations on some forums. If you could provide a report to replicate the issue I could tacle it more properly

On Fri, Jun 23, 2023, 04:58 Zachary Norman @.***> wrote:

Sorry if I wasn't clear before, but its still an issue.

Webpack can't resolve the HTTP or HTTPS lib dynamically from a variable name at runtime and is still giving me an error:

rejected promise not handled within 1 second: Error: Cannot find module 'https' extensionHostProcess.js:105 stack trace: Error: Cannot find module 'https' at webpackContextResolve (c:\Users\ \dist\apps\client\main.js:17839:11) at webpackContext (c:\Users\ \dist\apps\client\main.js:17834:11) at sendRequest (c:\Users\ \dist\apps\client\main.js:17504:41) at c:\Users\ \dist\apps\client\main.js:17778:13 extensionHostProcess.js:105

To be more clear, it needs an explicit import like this to work:

[image: image] https://user-images.githubusercontent.com/31664668/248142296-9d2c29a0-bf26-43f9-a5f7-070bcd60283a.png

I'm not an expert at webpack, so there might be some other kind of config I need, but when I use the explicit import I'm able to send events and have it work just fine.

— Reply to this email directly, view it on GitHub https://github.com/analytics-debugger/ga4mp/issues/20#issuecomment-1603618601, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALM4JAB5HSH3WLITXYYZOLXMUA45ANCNFSM6AAAAAAZPMKWLM . You are receiving this because you modified the open/close state.Message ID: @.***>

znorman-harris commented 1 year ago

Well I'm not sure what else to provide apart from my error and resolution above? Webpack is happiest with imports that are resolved ahead of time and not at runtime. So, when I make the changes to your lib, I no longer have warnings and am able to use it as expected.

znorman-harris commented 1 year ago

FYI I rewrote this in TypeScript and fixed the problem I was encountering. If you are interested in the source, here's the class for sending analytics and the logic for sending HTTP requests.

One thing that I also had occasionally were timeouts which had rejected promises, so here I'm wrapping the async HTTP get request in a try/catch so there are no bad errors.

https://github.com/interactive-data-language/vscode-idl/blob/7a748371a79b13abf12886f4cb6ae0cd24bc4773/libs/usage-metrics/src/lib/ga4/ga4-client.class.ts#L294

https://github.com/interactive-data-language/vscode-idl/blob/main/libs/usage-metrics/src/lib/ga4/helpers/send-request.ts