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

Provider a commonjs entry point for webpack, browserify or any other commonjs module bundler #440

Merged
merged 2 commits into from
Oct 20, 2015

Conversation

chentsulin
Copy link
Contributor

  • build js files from es6 to es5 into lib folder
  • change commonjs entry point to lib/sweetalert.js

Fix #417
@t4t5
Please checkout this, thx!

If everything is ok, please merge and bump the version. Thanks for your great work on this lib.

@idolize
Copy link

idolize commented Sep 24, 2015

@t4t5 Any chance of getting this reviewed?

@MikaAK
Copy link

MikaAK commented Oct 9, 2015

+1

@arianitu
Copy link

+1 what is going on with this?

@chibicode
Copy link

👍 @t4t5

@neighborhood999
Copy link

@t4t5 👍 +1

@chentsulin
Copy link
Contributor Author

@t4t5 Any update on this?

@idolize
Copy link

idolize commented Oct 19, 2015

I think either the author is on vacation or this library isn't being actively maintained anymore 😢

@t4t5
Copy link
Owner

t4t5 commented Oct 20, 2015

Sorry for my absence people, I've just been busy. This looks good and should be ready for merging. 👍 My only question is whether there's a reason why the sweetalert.js is in a new lib-folder instead of the existing dist-folder?

@chentsulin
Copy link
Contributor Author

@t4t5 It's just a convention. Some of projects (eg. React, Redux) put source code (es6, es7) in src, compiled es5 in lib, umd and minified files in dist.

@t4t5
Copy link
Owner

t4t5 commented Oct 20, 2015

@chentsulin Oh I see. Seems like a good convention. Merging now.

@t4t5 t4t5 merged commit ca740e1 into t4t5:master Oct 20, 2015
@chentsulin
Copy link
Contributor Author

@t4t5 why a .npmignore in the published module (1.1.1)?

*.codekit
*.sass-cache
*.DS_STORE
node_modules
bower_components
lib

lib in .npmignore will break this module. Must remove it.

@chibicode
Copy link

@chentsulin 👍 @t4t5

@adamrneary
Copy link

+1 to what @chentsulin is saying. we're getting Module not found: Error: Cannot resolve module 'sweetalert'because package.json is pointing to lib, but lib is being ignored: http://cl.ly/image/2j1T0b2y461D

@chentsulin
Copy link
Contributor Author

If a .npmignore is needed, I think we should also put it into this git repo.

@t4t5
Copy link
Owner

t4t5 commented Oct 21, 2015

Sorry about that. Seems like .npmignore picks up the contents from .gitignore, which had a "lib"-line there for some weird reason. Fixed in 1.1.2!

@chentsulin
Copy link
Contributor Author

My bad, there is still one more thing I forgot:

https://github.com/t4t5/sweetalert/blob/master/dev/sweetalert.es6.js#L68

@t4t5 Can we just add a export default here? This one works in CommonJS env (tested in webpack).

@t4t5
Copy link
Owner

t4t5 commented Oct 21, 2015

@chentsulin lol, really bumping those version numbers today. Everything working now? :)

@chentsulin
Copy link
Contributor Author

@t4t5 It works great now. Your help was greatly appreciated !!!

@t4t5
Copy link
Owner

t4t5 commented Oct 21, 2015

@chentsulin Oh I did nothing, thank YOU good sir for helping out! 😀

@chibicode
Copy link

@t4t5 thank you!

@alexsandro-xpt
Copy link

Thank you!! 😅

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.

None yet

9 participants