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

Modules in use now shows only valid modules #42

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Modules in use now shows only valid modules #42

wants to merge 2 commits into from

Conversation

hemanth
Copy link

@hemanth hemanth commented Mar 23, 2014

Potential fix for #41

@maxogden Tried using jsonp module, but that is failing saying it can't find the script tag!

So had to use this work around, now the 'modules in use' section lists only those modules that are present in npm registry.

It would be better if we highlight those which are invalid?

Sample Output:
screen shot 2014-03-23 at 14 11 06

@max-mapper
Copy link
Owner

nice, a couple pieces of feedback:

  • I run a cors proxy that I can re-deploy etc at http://cors.maxogden.com, so i'll probably change to that
  • we shouldnt technically need a cors proxy since couch supports cors natively as of couchdb 1.3
  • I wanna make sure this works if npm/the cors proxy is down. or at least shows an error message
  • I also agree that we should show errors somehow for the modules that don't exist

@hemanth
Copy link
Author

hemanth commented Mar 24, 2014

  • Tried this and got a response as: Missing required request header. Must specify one of: origin,x-requested-with am I missing soemthing?
  • Yes, couchdb 1.3 has Experimental support for CORS, but that is not enabled by default, right?
  • Error message will be taken care of.
  • Was pawing at it.

@max-mapper
Copy link
Owner

http://cors.maxogden.com/http://isaacs.iriscouch.com/registry/foo is the correct URL, the error message is because a GET via web browser address bar isn't a proper CORS request

I was told by @indexzero that the nodejitsu registry couch was supposed to have CORS turned on, but http://requirebin.com/?gist=maxogden/9735966 (hit run, look in console) isn't working

@hemanth
Copy link
Author

hemanth commented Mar 24, 2014

Oh ok!

Yes http://registry.nodejitsu.com/registry does not support CORS yet, so was using that jitsu proxy.

require('browser-request')('http://cors.maxogden.com/http://isaacs.iriscouch.com/registry/foo', console.log.bind(console)) worked well :) will be making the chances soon and if @indexzero could get the CORS turned on, nothing like it! Thanks.

@hemanth
Copy link
Author

hemanth commented Mar 24, 2014

@maxogden using cors.maxogden.com I noticed the below in FF:
screen shot 2014-03-24 at 18 04 25

@max-mapper
Copy link
Owner

@hemanth I just upgraded to latest version of https://www.npmjs.org/package/cors-anywhere and deployed and now it works in my FF, does it work in yours?

Maybe what we should do is show all the modules as they get parsed, like we do now, but optimistically try to check each one (maybe put a little loading spinner on them) against NPM. If the request fails or succeeds and it is in NPM then it keeps showing the module, if the request succeeds and it is not in the registry it removes the module or e.g. shows a red button instead of a green button

@hemanth
Copy link
Author

hemanth commented Mar 26, 2014

@maxogden After updating cors-anywhere it works fine, but looks like it's firing a request for every keystroke!

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.

2 participants