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

Fix importing issue with Webpack #42

Merged
merged 1 commit into from
Jul 9, 2022
Merged

Fix importing issue with Webpack #42

merged 1 commit into from
Jul 9, 2022

Conversation

henrikra
Copy link
Contributor

@henrikra henrikra commented Jul 8, 2022

Description

I noticed in this issue that importing this library does not work with Webpack correctly #41

Solution

The solution is to remove browser field because it was pointing to version of global ExtPay which is meant to be used when you want to have global variable ExtPay. But since package.json is meant for npm modules this is not what we want to do.

Very big libraries which are meant for browser do no use browser field of package.json. Great example from Vue.js (https://github.com/vuejs/vue/blob/main/package.json#L6). They only specify main + other fields but nobody uses browser.

How to test?

Use ExtPay library in Webpack and Rollup or some other bundler and see if you can now just import ExtPay like this

import ExtPay from 'extpay';

const extPay = ExtPay("some-id");

and run the code to see if you get any errors. If you don't then everything is cool!

Fixes #41

@henrikra
Copy link
Contributor Author

henrikra commented Jul 9, 2022

@Glench There are basically two options to fix this issue:

  1. Remove the browser field like I did in this PR
    OR
  2. Keep browser field but change its value to ./dist/ExtPay.module.js

I am fine with either 👍

Copy link
Owner

@Glench Glench left a comment

Choose a reason for hiding this comment

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

I'm a little wary that this could break other build systems, but we'll try it out for now and see if it creates any issues.

@Glench Glench merged commit e4cc661 into Glench:main Jul 9, 2022
@henrikra
Copy link
Contributor Author

It won't 👍

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.

Importing ExtPay does not work with Webpack
2 participants