Add filename info to mdx loader #249
Conversation
|
This pull request is automatically deployed with Now. To access deployments, click Details below or on the icon next to each push. |
CompuIves
force-pushed
the
add/filename-info
branch
3 times, most recently
from
1a6e905 to
ef2c059
Compare
wooorm
left a comment
wooorm
left a comment
There was a problem hiding this comment.
Looks good, I have a few questions/tips though. Let me know what you think!
packages/mdx/index.js
Outdated
| if (opts.filename) { | ||
| fileOpts.path = opts.filename; | ||
| } | ||
| const file = vfile(fileOpts); |
There was a problem hiding this comment.
On thing that may make this cleaner, and remove the vfile dependency, is that you pass fileOpts directly into .process / .processSync, no need to wrap yourself!
There was a problem hiding this comment.
Hmmmmm that's true, that would make the API not backward compatible though because it would need to be wrapped in {content}. It feels a bit less straightforward, but I can change it!
There was a problem hiding this comment.
would it not? .process and .processSync accept both, and create a vfile from either? Maybe I’m missing something!
There was a problem hiding this comment.
Whoops, you're right!
|
Bedankt voor de review! I'll process the comments this evening 😄 |
CompuIves
force-pushed
the
add/filename-info
branch
2 times, most recently
from
7afca58 to
18644ff
Compare
|
Hey! I processed the parameter to Never mind! That was my mistake, I processed all feedback now! |
CompuIves
force-pushed
the
add/filename-info
branch
2 times, most recently
from
1b95128 to
9c4d7b8
Compare
CompuIves
force-pushed
the
add/filename-info
branch
from
9c4d7b8 to
dcb608b
Compare
| const compiler = createCompiler(opts) | ||
|
|
||
| const { contents } = compiler.processSync(mdx) | ||
| const fileOpts = { contents: mdx }; |
There was a problem hiding this comment.
This looks amazing! You could pass {contents: mdx, path: opts.filepath}, but that's just nitpicking.
Looks great, lekker bezig!
There was a problem hiding this comment.
Hmm yes, I didn't do it in this case since the mdx transpiler didn't work for me when path: undefined. I can still change it since that's probably something that won't often happen, but this one is more correct methinks.
Also, dankjewel :D! Leuk om zoveel Nederlanders hier te zien!
There was a problem hiding this comment.
Weird! Well then it’s fine! En ja, klopt! 👍
|
Thanks, this looks great! |
3 participants
For a rehype plugin I'm working on I need the path of the current file being transpiled, I use it to resolve imports. This adds the filename info to the
processfunction which allows plugins to access it.