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

Expose lodash to node-webkit #419

Closed
wants to merge 2 commits into from
Closed

Expose lodash to node-webkit #419

wants to merge 2 commits into from

Conversation

jawb
Copy link

@jawb jawb commented Dec 4, 2013

No description provided.

@jdalton
Copy link
Member

jdalton commented Dec 4, 2013

I'm all for added support.
@jawb can you explain what the issue is, why the patch is done the way it is, and sign the cla.

@jawb
Copy link
Author

jawb commented Dec 4, 2013

in node-webkit lodash is not exposed, I modified the condition

@jdalton
Copy link
Member

jdalton commented Dec 4, 2013

On further review I believe this is a dup of #363.

@jdalton jdalton closed this Dec 4, 2013
@jawb
Copy link
Author

jawb commented Dec 4, 2013

The problem is not when using require but when including _ using script tag.

@jdalton
Copy link
Member

jdalton commented Dec 4, 2013

Based on the usage examples it looks like using the script tag is not what's promoted. Use the node require syntax for this.

@jawb
Copy link
Author

jawb commented Dec 4, 2013

Yeah, but I don't generate the .nw every time I just use script tag to use refresh and make testing easier

@jdalton
Copy link
Member

jdalton commented Dec 4, 2013

If I modified our exports then it would break the common usage for node-webkit. For your needs you could run the custom build tool to specify that you only want to export to the global via:

lodash mobile exports=global

@jawb
Copy link
Author

jawb commented Dec 4, 2013

Sure, that's a good solution

@tomasdev
Copy link

👍 is there a way to make whatever exports=global does, without breaking anything, for the regular builds?

I mean, it took me 2hs to finally find this post, could be nice to have it integrated in the next version :P

@terinjokes
Copy link
Contributor

@tomasdev Not that I know of. Including exports=global in the build means the output only have the global export option, whereas the regular build has exports for Node/CommonJS, AMD and then global.

@tomasdev
Copy link

Then I would consider this as an issue for webkit-node where the "automatic switching context" of V8 is not doing its job properly.

BTW, I think we could do ifs instead of if-elses

@tomasdev
Copy link

Also, using the UnderscoreJS library doesn't trigger this error, so I guess the fix needs to be part of the "as drop in of underscore replacement" version.

jdalton added a commit that referenced this pull request Jan 26, 2014
@jdalton
Copy link
Member

jdalton commented Jan 26, 2014

@tomasdev Thanks. I dug in a bit more and found that jQuery and Underscore work without issue so I hammered on it a bit and fixed it so Lo-Dash works out of the box with node-webkit too 😀

@terinjokes
Copy link
Contributor

@jdalton Nice.

@tomasdev
Copy link

Yay!! Open source FTW. Thank you very much.

ghost pushed a commit to bestiejs/json3 that referenced this pull request Jan 26, 2014
See lodash/lodash#419.

Remove the now-unused `isHostType` function.
@jasonfutch
Copy link

Where would that file be? The ones on the website still don't work but underscore does.

@jdalton
Copy link
Member

jdalton commented Feb 2, 2014

@jasonfutch
It will be available on cdns & our site on the next version bump.
You can get edge versions of the files atm here:
https://github.com/lodash/lodash/tree/master/dist

jdalton added a commit that referenced this pull request Sep 25, 2014
@lock
Copy link

lock bot commented Dec 28, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

5 participants