alexgibson / notify.js

A handy wrapper for the Web Notifications API
https://alexgibson.github.io/notify.js/
Other
1.33k stars 148 forks source link

Weird export since 2.x #66

Closed marudor closed 7 years ago

marudor commented 8 years ago

Hey, I've updated to 2.0.2 and noticed that it exports differently. I'm using Babel + CommonJS Module Plugin. Since 2.0.2 I have to do the following:

import Notify from 'notifyjs';
if (Notify.default.needsPermission) {
  Notify.default.requestPermission();
}

Notice the default. - apparently it exports a Object that has a default member as default.

alexgibson commented 8 years ago

Yeah this is expected when the babel transpiles. You can just reference default when importing rather than need to define it explicitly every time.

alexgibson commented 8 years ago

Fwiw, the source is now writen in ES6 too if you want to import directly.

marudor commented 8 years ago

Actually it shouldn't. Notifyjs has 2 defaults. It should only have 1. Thats probably a problem with jspm though.

JSPM exports an object like this:

{
   default: NotifyJS
}

ES6 expects this though:

{
    default: NotifyJS,
    __esModule: true
}

If the esModule is not set babel will wrap it into a default. So we need the esModule property.

alexgibson commented 8 years ago

Hmm interesting, I will take a look into this to see if it's something possible in jspm. Thanks

marudor commented 8 years ago

This is what babel adds to a export

Object.defineProperty(exports, "__esModule", {
  value: true
});
alexgibson commented 8 years ago

This may also be relevent: http://stackoverflow.com/questions/33704714/cant-require-default-export-value-in-babel-6-x

marudor commented 8 years ago

That applies only to "normal" requires though, not to ES6 import. The import handles the default thing.

alexgibson commented 8 years ago

Yeah, right now I'm unclear if this is something that jspm can fix, but will continue looking.

Does something like import { default as Notify } from 'notifyjs'; work?

marudor commented 8 years ago

It does. It's a workaround.

alexgibson commented 8 years ago

Well, according to the ES6 spec a default export is no different to a named one, so it's pretty much sugar. But I agree the old way was nicer. This is a little unusual as you're importing something that's already been transpiled (as opposed to importing the ES6 src). But I'll see what I can work out. Thanks for filing.

ahutchings commented 8 years ago

The import { default as Notify } from 'notifyjs'; workaround does not work for me when I install notifyjs via npm. The final Notify reference has two levels of objects with default properties as @marudor mentioned.

It may be worth looking at a project like reactjs/redux to see if notify.js can reuse their build config - it is authored with ES6 modules, but transpiled down to commonJS.

alexgibson commented 8 years ago

I'm going to look at removing the dependency on jspm in the future. I may look at rollup as an option.

ahutchings commented 8 years ago

Sounds good. I may take a stab at the rollup build unless you get there first.

alexgibson commented 8 years ago

Pull requests always welcome :)

alexgibson commented 7 years ago

This should hopefully be fixed shortly by #73

alexgibson commented 7 years ago

Fixed by #73