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

Eleventy plugin not working #51

Closed
dpfavand opened this issue Jun 28, 2021 · 6 comments · Fixed by #58
Closed

Eleventy plugin not working #51

dpfavand opened this issue Jun 28, 2021 · 6 comments · Fixed by #58
Labels
Bug Something isn't working

Comments

@dpfavand
Copy link
Contributor

Hello,

I'm trying to run this within 11ty but haven't gotten past the setup part, as the plugin errors out with Could not get Shiki loaded async via 'deasync'

Minimal reproduction: https://replit.com/@dpfavand/11ty-shiki-twoslash-doesnt-work (click the green button to run).

It appears that the setupForFile promise is never resolved or rejected. But that's about as far as I have been able to get.

Node 14 and 12
Eleventy 12.1
eleventy-plugin-shiki-twoslash 1.0.7

Has anyone else run into this lately? Running on Node 12 and 14, both promises and async should be available, so I'm not sure what's going on.

@orta
Copy link
Contributor

orta commented Jun 29, 2021

Unsure of your setup, but I just re-tried the example 11ty version in this repo and it works fine (node 15) https://github.com/shikijs/twoslash/tree/main/examples/eleventy

Perhaps replit is timing out due to hardware constraints?

@orta orta added the Bug Something isn't working label Jun 29, 2021
@dpfavand
Copy link
Contributor Author

dpfavand commented Jul 1, 2021

Thanks for checking the example. I've been doing some more testing, and seem to have gotten past that error and run into another. So I don't think it was hardware constraints - it's even stranger.

My local machine is WSL Ubuntu with Node 14 and 15 (via nvm).

I'm using your example repo and mine.

Notes

First test: Node 12 vs 14 vs 15

Locally I tested 14 and 15, and used 12 in the replit. Node 15 worked, but 12 and 14 didn't. This was disappointing because 14 is LTS and I was hoping this library would work on Node LTS. However, then I tested something else - the Eleventy build command.

npx @11ty/eleventy vs eleventy

I noticed that my initial build command followed the current Eleventy getting started guide: npx @11ty/eleventy. I set this as the build script in my package.json. On a whim I swapped it out for a more traditional eleventy command. And it worked! On both my local Node 14 and Replit's Node 12. Mostly...

New error: Cannot read property 'split' of undefined

Although calling the build process through eleventy rather than npx @11ty/eleventy got past the async load error in my original issue, my repository runs into a different error, Cannot read property 'split' of undefined at https://github.com/shikijs/twoslash/blob/main/packages/remark-shiki-twoslash/src/index.ts#L35 - it appears that metaString is undefined at that point.

This occurs on my example in Node 12 and 14, but not in your example on Node 14. Your example also uses what I think is an old canary version of Eleventy. I tested changing your example to Eleventy 0.12.1 and it shows the same error, on both Node 14 and 15.

Conclusions

  • For whatever reason, npx @11ty/eleventy behaves differently than calling eleventy directly on Node 14. I don't understand why, but basically on Node 12 and 14 you cannot build using npx @11ty/eleventy when you have this plugin configured. Not a huge deal in the end but definitely confusing.
  • The plugin does not (in my testing) appear to work with the current 12.1 release of Eleventy, with the undefined metaString issue. I will investigate further, perhaps the Eleventy plugin API changed since the canary version used in your example. If I find something I'll post and/or try to make a PR.

The Replit example linked above is updated and exhibits the same behavior as my local Node 14 and 15 environments using Eleventy 12.1.

@dpfavand
Copy link
Contributor Author

dpfavand commented Jul 1, 2021

Actually it looks like the pinned canary version is probably ahead of the current release? So likely this plugin actually requires functionality not in the current 0.12.1 Eleventy release. If that is the case, would you accept a documentation PR to make that clearer?

@frencojobs
Copy link
Collaborator

frencojobs commented Jul 2, 2021

Thanks a lot for the thorough explanation, it really helps us.

I think npx runs the latest LTS release not the canary version of eleventy to run the command. So the only problem we have here is that Cannot read property 'split' of undefined error which is caused by the third param to highlighter not being available in older versions of markdown-it which eleventy before v1 uses.

eleventyConfig.addMarkdownHighlighter((code, lang, fence) => {

markdown-it/markdown-it#626 (comment)

We might need a workaround to get the fence in older versions or we could just drop the support for eleventy v0.*s. But eleventy v1 is currently in alpha and not many ppl use it so it's not optimal. Now I'm looking for a workaround, and if the workaround is not possible or clashing with the latest versions, we might just have to drop the support then.

@frencojobs
Copy link
Collaborator

frencojobs commented Jul 2, 2021

Update here. I don't think it is possible. To override the default behavior of older markdown-it, we need to get access to the MarkdownIt() instance, which in our case is this.mdLib from the eleventy's markdown engine, which I don't think is possible to access through the config object, which is the only thing we have access to.

Long sentence short, even the official syntax highlighters use this weird syntax to highlight lines because it's impossible to get attributes other than the first one.

	  ```js/1,3
	  console.dir(console);
	  ```
	  
	  👇 one plugin use this syntax
	  
	  ```js#13
	  console.dir(console);
	  ```
let highlights = new HighlightLinesGroup(split.join("/"), "/");

If that is the case, would you accept a documentation PR to make that clearer?

Yes, PRs are welcomed, and thanks.

@dpfavand
Copy link
Contributor Author

dpfavand commented Jul 2, 2021

Thanks for digging in and the explanation. I've submitted #58 to update the plugin readme.

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants