Escape invalid URL symbols

[ibc]

Currently embed-js "fails" to detect this URL:

https://whatever.com?x={thisfails}

This is "correct" given that { and } are not valid symbols within the query parameters of a URL. However if you paste the above URL into any browser, it will properly encode them, which is the same as doing this:

encodeURI('https://whatever.com?x={thisfails}')

=> "https://whatever.com?x=%7Bthisfails%7D"  // VALID URL

The point is that embed-js may allow them.

[ritz078]

The point still remains that it's not a valid URL so what's the point in creating a wrong URL that lands you to nothing

[ibc]

Most browsers allow typing a "wrong" URL and they properly encode it before sending the HTTP request.

Following your logic, typing http://españa.com is also invalid (and in fact, embed-js does not process it as a link). Now just open http://españa.com in your browser. It will transparently convert it into http://xn--espaa-rta.com while still displaying http://españa.com in the URL bar. Then I copy it, process it with embed-js and it fails.

I strongly think embed-js should do the magic to handle those URLs.

[ibc]

what's the point in creating a wrong URL that lands you to nothing

Just curious: the Markdown used by github allows the first "wrong" URL but not the second one:

• http://whatever.com?x={thisfails}
• http://españa.com

[ritz078]

I opened http://españa.com in Chrome and even the URL changes

[ibc]

It's not changes in my Chrome (OSX), but that's irrelevant. The question here is whether embed-js should allow those links or not. The fact that http://españa.com is changed or not in your browser does not invalidate my previous rationale.

BTW: if you paste http://españa.com or http://whatever.com?x={thisfails} into OSX Mail app (when writing an email) it's also detected as a link.

[ibc]

If this issue is just "invalid" (although it's not), can it be closed please?

Just to summarize:

• Open this "invalid" URL by clicking into it:

http://españa.com

• It seems that it works, right?
• Now paste it in embed-js. It will fail by displaying this wrong link:

http://españa.com

[ritz078]

How about sending a PR ?

[ibc]

That may work. Just please confirm that it would be accepted (if nothing is broken, of course).

[ritz078]

yes definitely. If all the currently supported URLs also work and nothing breaks, I will definitely accept it. :)

[ibc]

Great. Let me work on it and will submit a PR.

[ibc]

Let me a question please: which grunt task should I run in order to test the generated library? I'm a bit confused. It seems that both gulp build and gulp dist produce the src/embed.js bundled library.

[ritz078]

Just run grunt.

[ibc]

Thanks.

[ibc]

Any tip with this please? I've properly installed NPM modules and global rollup:

$ grunt
Running "eslint:target" (eslint) task

Running "shell:rollup" (shell) task
Cannot find module '__babelHelpers__' from '/Users/ibc/src/embed.js/src/js/modules'
Error: Cannot find module '__babelHelpers__' from '/Users/ibc/src/embed.js/src/js/modules'
    at /Users/ibc/src/embed.js/node_modules/rollup-plugin-npm/node_modules/resolve/lib/async.js:46:17
    at process (/Users/ibc/src/embed.js/node_modules/rollup-plugin-npm/node_modules/resolve/lib/async.js:173:43)
    at ondir (/Users/ibc/src/embed.js/node_modules/rollup-plugin-npm/node_modules/resolve/lib/async.js:188:17)
    at load (/Users/ibc/src/embed.js/node_modules/rollup-plugin-npm/node_modules/resolve/lib/async.js:69:43)
    at onex (/Users/ibc/src/embed.js/node_modules/rollup-plugin-npm/node_modules/resolve/lib/async.js:92:31)
    at /Users/ibc/src/embed.js/node_modules/rollup-plugin-npm/node_modules/resolve/lib/async.js:22:47
    at FSReqWrap.oncomplete (fs.js:82:15)
Type rollup --help for help, or visit https://github.com/rollup/rollup/wiki

[ibc]

Installing babel-helpers does not solve the problem. Found a similar https://github.com/rollup/rollup-plugin-babel/issues/60, but I'm not used to rollup so I have no idea.

[ibc]

Just this command produces the error:

$ rollup -c rollup.umd.config.js

Error: Cannot find module '__babelHelpers__' [...]

[ritz078]

use rollup-plugin-babel v2.4.0 and then try

[ibc]

yep, it works!

I will modify the version in my forked package.json to be "2.4.0". Thanks.

[ibc]

It neither detects http IPv6 links... I will also add it.