stampit-org / stampit

OOP is better with stamps: Composable object factories.
https://stampit.js.org
MIT License
3.02k stars 102 forks source link

UMD build in browser environment doesn't work actually #259

Closed danielkcz closed 7 years ago

danielkcz commented 7 years ago

We have never tested that apparently. The problem is, that window.stampit gets assigned the exports object instead of actual stampit function. Then anyone actually trying to use stampit() won't succeed.

Obviously it's a rollup thing, but kinda understandable. I've created the issue there with possible solution, so we will see what happens.

In case they would be against it, we should add warning to the README that stampit.default needs to be used. I assume that not many people actually uses libraries directly like this anymore, so I it's not a big problem I would say.

danielkcz commented 7 years ago

Ok, based on comment in linked issue we will need to add extra entry point that will be used solely for UMD builds and export stampit as default only. Anyone wants to learn something new? Should be probably fairly easy.

koresar commented 7 years ago

Took a read. Thought a lot.

Why stampit need ES6 exports? The only useful feature I see is the tree-shaking. However, it's not beneficial for a small module like stampit to provide tree-shaking. There is no gain.

My proposal: Ditch the ES6 exports syntax. And simply module.exports = stampit. As the result:

But! We would need to revive ES6 exports in the future @stamp/stamp project.

What do you think?

danielkcz commented 7 years ago

Well I am using named exports in my project, it just feels better if I can write directly init(...) instead of stampit.init(...) :) It's also breaking change, so I wouldn't really go that way anytime soon. Extra entry point is lesser evil.

koresar commented 7 years ago

Oh. You mean import init from 'stampit' won't work any more if we leave this line only? module.exports = assign(stampit, allUtilities);

danielkcz commented 7 years ago

import init from 'stampit' is same as import stampit from 'stampit', just different name for a default export. You will always get stampit function so I would need to do init.init() :D

You cannot really use import to retrieve only one property of default export, that's not how it works because ESM are completely lexical, they cannot statically analyze what you are exporting.

koresar commented 7 years ago

Surely I meant {init} :)

danielkcz commented 7 years ago

Yea, that wont work either obviously :) You can check it out here

anhuiliujun commented 7 years ago

So, if i am not using any bundle system, how should i use stampit now?

koresar commented 7 years ago

In the README.md we say about unpkg.com a https://unpkg.com/stampit@latest/dist/stampit.umd.min.js and cndjs.com https://cdnjs.com/libraries/stampit

Did those UMD bundles worked for you?

anhuiliujun commented 7 years ago

Thank you very much, and sorry for the noise, I forgot the object.assign polyfill.

koresar commented 7 years ago

No worries.

koresar commented 7 years ago

I tested. UMD work now. Here is the installation instructions: https://github.com/stampit-org/stampit#install

I'm closing this issue. It looks done long ago :)