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

Import URL explicitly to work in node 8 #80

Merged
merged 3 commits into from
Feb 2, 2020

Conversation

ilyamkin
Copy link
Contributor

@ilyamkin ilyamkin commented Feb 2, 2020

What:
Import URL module explicitly. Fixes #61

Why:
In order to make the plugin work in Node 8.

How:
Apart from necessary import, I've added a condition in catch to suppress only TypeErrors (invalid links). It would make it easier to debug for those who are using old Node versions.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

Copy link
Owner

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyamkin Seems like tests are failing now.
Could you take a look at them please?

@MichaelDeBoey
Copy link
Owner

MichaelDeBoey commented Feb 2, 2020

@ilyamkin Could you also add Node 8 to .travis.yml then please?
Just to be consistent.
I think this is the reason why the tests were failing on Node 8 when I added the plugin tests (#27 (comment))

@ilyamkin
Copy link
Contributor Author

ilyamkin commented Feb 2, 2020

All tests are passing now (including node 8). There is one new line that I've added, so coverage is not 100% anymore. Wondering how I can test it.

@MichaelDeBoey
Copy link
Owner

Hmmm lets just always suppress the error, like we did before.
new URL() is only gonna throw an TypeError anyways so I don't mind.

@codecov-io
Copy link

codecov-io commented Feb 2, 2020

Codecov Report

Merging #80 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #80   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          14     14           
  Lines         160    160           
  Branches       14     14           
=====================================
  Hits          160    160
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27ceacf...754a6c6. Read the comment docs.

@MichaelDeBoey MichaelDeBoey self-assigned this Feb 2, 2020
Copy link
Owner

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! 👊

@MichaelDeBoey MichaelDeBoey merged commit 203201a into MichaelDeBoey:master Feb 2, 2020
@MichaelDeBoey
Copy link
Owner

@all-contributors Please add @ilyamkin for code

@allcontributors
Copy link
Contributor

@MichaelDeBoey

I've put up a pull request to add @ilyamkin! 🎉

@MichaelDeBoey
Copy link
Owner

🎉 This PR is included in version 1.11.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

anuraghazra pushed a commit to anuraghazra/gatsby-remark-embedder that referenced this pull request Feb 5, 2020
* fix: import URL explicitly to work in node 8

* fix: rely on error code, add node 8 to travis

* fix: always suppress the error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

youtube embed not working once deployed to netlify
3 participants