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

The word "default" anywhere in the meta description causes an incorrect error #341

Closed
jescalan opened this issue Nov 29, 2018 · 4 comments · Fixed by #362
Closed

The word "default" anywhere in the meta description causes an incorrect error #341

jescalan opened this issue Nov 29, 2018 · 4 comments · Fixed by #362
Labels
🐛 type/bug This is a problem

Comments

@jescalan
Copy link
Contributor

This is the line that's causing the issue. To reproduce, just include the word "default" anywhere in the metadata. For example:

export const meta = {
  title: "Testing 123",
  description: "This is a default description"
}

# Content here

Again very happy to submit a patch, just want to get this on the maintainer(s) radar

@johno johno added the 🐛 type/bug This is a problem label Nov 29, 2018
@johno
Copy link
Member

johno commented Nov 29, 2018

Thanks for the bug report. We'd gladly accept a patch!

@jescalan
Copy link
Contributor Author

jescalan commented Nov 29, 2018

@johno Fantastic! I'll have a patch in shortly. I'm just going to modify the regexes to be a little more precise in picking out the correct patterns.

Another option for long term stability would be to run an actual parse using something like babel-parser to pull out the tokens, rather than regexes. This is the second regex parsing bug I have turned up in my initial experiments, and I'm sure there are more lurking. I suppose the downside here would be a performance hit. If you're interested, I'd be happy to start tinkering with a patch for this as well and run a perf comparison.

@johno
Copy link
Member

johno commented Nov 29, 2018

Thanks @jescalan!

Yeah, we might have to end up going down the formal parser path for handling import/export/jsx nodes. Though I'm hoping to hold out for micromark because it'd be able to handle quite a bit more of the tokenization.

One of the big remaining parsing issues we have is not handling JSX blocks with empty lines in them, which we would be able to trivially implement with babel by continuing to "walk" the document when a JSX block opens until its parseable.

@jescalan
Copy link
Contributor Author

Yeah I was thinking it would be used only for the import/export portion of the file initially, not the markdown content. In my logging around, it seemed like there is a spot in the code where the es6 code is separated out from the markdown content. That might be fairly quick to run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 type/bug This is a problem
Development

Successfully merging a pull request may close this issue.

2 participants