Skip to content

Conversation

@jasononeil
Copy link
Contributor

This enables us plugins to be set in Webpack v1 where configuration can only be given as JSON serialisable values, so functions don't work.

(Reason I'm still using Webpack v1: I'm using Gatsby as a static site generator and they haven't upgraded yet)

This enables us plugins to be set in Webpack v1 where configuration can only be given as JSON serialisable values, so functions don't work.

(Reason I'm still using Webpack v1: I'm using Gatsby as a static site generator and they haven't upgraded yet)
@codecov-io
Copy link

codecov-io commented Dec 22, 2017

Codecov Report

Merging #27 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           develop    #27   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files            9      9           
  Lines          119    122    +3     
  Branches        25     25           
======================================
+ Hits           119    122    +3
Impacted Files Coverage Δ
src/convert.js 100% <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 579fad7...e354952. Read the comment docs.

@jasononeil
Copy link
Contributor Author

Well, I tried to add a fixture to test that we have coverage on the "specifying MarkdownIt plugins as strings" functionality, but I'm having trouble reading the diff in the Jest snapshot file to see if it's had adverse affects. Does it seem okay to you @ticky or does the giant snapshot diff mean I've broken something?

It's still working well for me on my local tests

@ticky
Copy link
Owner

ticky commented Jan 4, 2018

The giant snapshot diff is really difficult to understand, sorry about that. I got a bit over-excited at the potential and unfortunately it’s gotten a bit out of hand.

I’ve moved your additional spec to be its own individual spec, which now means there’s only a single block addition to the snapshot, which is a bit more clear. Your code didn’t break anything - good stuff!

I’ve updated the Readme a bit, but I don’t have a Webpack 1.x project on hand to test - can you confirm whether you can use require.context alone, or a combination of path.relative and require.resolve as in the spec? I’ve added a paragraph about this: https://github.com/jasononeil/markdown-component-loader/tree/53c2a489fc04ff98b8d4e09f1a42c5aaf11d7818#legacy-webpack-compatibility

Copy link
Owner

@ticky ticky left a comment

Choose a reason for hiding this comment

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

Pending confirmation that the readme does not lie, I’m feeling happy to merge this and cook you up a release with it! 😄

@jasononeil
Copy link
Contributor Author

Thanks for the clean-up, so much better! (And sorry it's taken me so long to get back to you).

I can confirm that the README is correct and is working on my Webpack 1 Gatsby project.

Thanks again, looking forward to the release so I can stop having a git-fork in my package.json 😄

@ticky
Copy link
Owner

ticky commented Jan 24, 2018

Thanks for the confirmation! I’ll get this cooked up into a release imminently so you can update your references :)

@ticky ticky merged commit a2800d2 into ticky:develop Jan 24, 2018
@ticky
Copy link
Owner

ticky commented Jan 24, 2018

Just published in v1.1.0. :)

@jasononeil
Copy link
Contributor Author

🎉 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.

3 participants