Glench / ExtPay

The JavaScript library for ExtensionPay.com — payments for your browser extensions, no server needed.
https://extensionpay.com
Other
483 stars 62 forks source link

Importing ExtPay does not work with Webpack #41

Closed henrikra closed 2 years ago

henrikra commented 2 years ago

Describe the bug A clear and concise description of what the bug is.

I have a working Chrome extension which uses TypeScript and it makes the build with Webpack. But unfortunately when I am following the instructions of ExtPay I cannot import the ExtPay function in any way. Below there are two things I have tried:

// in background script file

import ExtPay from 'extpay';
const extpay = ExtPay('sample-extension');

Error you will get in this case

Uncaught TypeError: extpay__WEBPACK_IMPORTED_MODULE_1___default(...) is not a function

According to the TypeScript types that this library provides this should be correct but still it doesn't work.

Then I tried another way shown in the README without any luck.

// in background script file

import 'ExtPay'
const extpay = ExtPay('sample-extension');

Error you will get

Uncaught ReferenceError: ExtPay is not defined

So I guess ExtPay tries to expose global object without being succesful. What is the reason to use globals in the first place? :P Usually they make things more complicated.

To Reproduce Steps to reproduce the behavior: Have a Chrome extension project with Webpack build and see it failing

So am I missing something from the docs or does this thing work with Webpack builds?

Glench commented 2 years ago

Can you please create an example repo that shows this happening with what commands to run? I'm not very familiar with webpack.

henrikra commented 2 years ago

Sure here is example repo I did: https://github.com/henrikra/demo-extpay-webpack-problem

  1. npm install
  2. npm run build to make compiled JavaScript
  3. Go to chrome://extensions/ and load the dist folder as unpacked extension

You will instantly error saying that the the imported values from extpay is not function.

Hope you can see the problem now 👍

Glench commented 2 years ago

When I run the commands in the repo you linked, I can't reproduce the bug. Instead what happens is the dist/ folder just has a bundle.js file with no manifest:

Screen Shot 2022-07-05 at 11 08 57 AM

henrikra commented 2 years ago

@Glench Please try again. I accidentally put manifest.json to .gitignore. Pull the changes and try again 👍

doriandrn commented 2 years ago

Hey there,

I had the same issue and upon checking the code in the "dist" dir, it seems there is no export defined for the main extpay js file.

A quick workaround to make this work is to import the .module file which has an export.

import ExtPay from 'extpay/dist/ExtPay.module'
Glench commented 2 years ago

So yeah, this is a configuration error with your webpack config. By default webpack tries to resolve libraries by their package.json's fields in this order: ['browser', 'module', 'main'] (source). If you want to use import, you want to do something like this instead:

webpack.config.js

module.exports = {
  //...
  resolve: {
    mainFields: ['module', 'main'],
  },
};

Does anyone know how other libraries do this? If there's a way I can make ExtPay work with webpack's default settings that would probably reduce a lot of confusion.

henrikra commented 2 years ago

I don't think the Webpack configuration is wrong. I have used Webpack around 6 years I had never a need to define resolve mainFields 🤔

This is the first library to counter this problem

henrikra commented 2 years ago

I think this is your problem. You should not distribute browser variant of ExtPay with npm package release. The browser field is pointing to the global version of ExtPay:

image
Glench commented 2 years ago

I think this is your problem. You should not distribute browser variant of ExtPay with npm package release. The browser field is pointing to the global version of ExtPay:

Hm, I don't know. In the package.json docs it says to define browser if your library is meant to be used client-side, which obviously this library is, which means it shouldn't have exports/imports. So it seems like kind of a case between what package.json defines and what webpack does by default. Confusing and I'm not sure what to do here if anything. I'm open to suggestions.

Regardless, there are multiple fixes for the problem in this thread so I may end up closing it.

henrikra commented 2 years ago

If check any other libraries inside of node_modules you can see that they don't use browser field in their package.json for this exact reason. Here is example of famous data-fns library (https://github.com/date-fns/date-fns) inside of my project and it doesn't have browser field. It has only main which is the correct way 👍

image
henrikra commented 2 years ago

Another big library called Vue (https://github.com/vuejs/vue) does not use browser field even though it is meant for browser:

image

So I would say it is very clear that the browser field should not be used

henrikra commented 2 years ago

@doriandrn and @Glench Can you test out the PR I did to fix this issue? https://github.com/Glench/ExtPay/pull/42

Glench commented 2 years ago

OK cool, thanks for finding examples and for the PR. I guess if possible I want to make sure that this change, which as far as I know seems to only impact webpack, doesn't break other build systems. Do you know any other resources on the "browser" field and can you explain where the discrepancy is between the official package.json docs and webpack's implementation?

henrikra commented 2 years ago

What discrepancy do you mean?

Btw check the PR, I added also a comment there 👍

Glench commented 2 years ago

The discrepancy between package.json saying to use the browser field for browser-based libraries and webpack failing to import a library using globals that's meant for the browser.

For now I'm just going to say YOLO and merge the change and we'll see if it breaks any other build systems.

henrikra commented 2 years ago

Just tested the new release and works like a charm!