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

Import broken in latest beta #583

Closed
henryruhs opened this issue Dec 19, 2019 · 11 comments
Closed

Import broken in latest beta #583

henryruhs opened this issue Dec 19, 2019 · 11 comments
Assignees
Labels

Comments

@henryruhs
Copy link

henryruhs commented Dec 19, 2019

Import via 3.0.0-beta.2 is broken as of missing index.js file.

Current import workaround:

import Cookies from 'js-cookie/dist/js.cookie';

Solution via node_modules/js-cookie/index.js:

module.exports = require('./dist/js.cookie');

or

module.exports = {
    Cookies: require('./dist/js.cookie');
};

Desired import:

import Cookies from 'js-cookie';
@henryruhs henryruhs added the bug label Dec 19, 2019
@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Dec 19, 2019

Can you write a test reproducing the issue so that we can guard against regression?

@henryruhs
Copy link
Author

henryruhs commented Dec 19, 2019

An import issue is not testable... especially for a dist that has been build.

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Dec 19, 2019

@redaxmedia

There is a test for ES6 import already here. What's the difference between the way you're using and they way it's being tested?

Could it be that we need a test for importing cookies from the root of the package so that it can trigger the need for the "main" field on package.json?

package.json:

{
  "main": "./dist/js.cookie"
}

@henryruhs
Copy link
Author

henryruhs commented Dec 19, 2019

Nope, this test does not take care of the dist package released on NPM. Just checkout the beta via npm install and try to import it inside a JavaScript file. I cannot provide more help at this point.

@carhartl
Copy link
Member

Also see: #544 (comment)

@henryruhs
Copy link
Author

henryruhs commented Dec 19, 2019

@carhartl I opened another issue cause this has nothing todo with the testing setup. Never mix up issues on one task / branch.

@carhartl
Copy link
Member

@redaxmedia Aye aye. It looked like it could be the same issue, but you seem to know better.

@carhartl
Copy link
Member

I‘m still confused as to how the index.js relates to the browser field in the package definition. Also, should index.js point to the es6 or umd dist variant. Can’t seem to find good answers on the internet.

@carhartl
Copy link
Member

Slightly different advice here: #544 (comment)

@carhartl carhartl self-assigned this Dec 20, 2019
@carhartl
Copy link
Member

@redaxmedia I've added an example where I've tested (in Nodejs 13.2.0) that this issue is now fixed, but could you also please give it a try...

@henryruhs
Copy link
Author

Finally, I can confirm that 3.0.0-beta.3 works again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants