Skip to content

chore(deps): upgrading @readme/syntax-highlighter#61

Merged
kellyjosephprice merged 2 commits intonextfrom
chore/update-syntaxhighlight
Nov 9, 2020
Merged

chore(deps): upgrading @readme/syntax-highlighter#61
kellyjosephprice merged 2 commits intonextfrom
chore/update-syntaxhighlight

Conversation

@erunion
Copy link
Copy Markdown
Member

@erunion erunion commented Oct 29, 2020

🧰 Changes

Been a while since the syntax highlighter has been updated here and since I added support for TOML to it today, we should get it up to date here so Markdown code blocks with that language should get highlighted.

📖 Release Notes

10.2.0 (2020-10-29)

10.1.2 (2020-10-26)

10.1.1 (2020-10-23)

10.1.0 (2020-10-22)

10.0.1 (2020-10-12)

10.0.0 (2020-10-05)

9.0.1 (2020-09-16)

9.0.0 (2020-09-15)

8.0.3 (2020-09-02)

  • chore(deps): upgrading @readme/variable to 7.2.1 (7ec28f3)

8.0.2 (2020-09-02)

  • docs: updating a link in the contributing docs (c539940)

8.0.1 (2020-09-01)

8.0.0 (2020-08-31)

No breaking changes in this release, the package has just moved a new home at https://github.com/readmeio/syntax-highlighter!

@erunion erunion added the dependencies Pull requests that update a dependency file label Oct 29, 2020
// https://github.com/codemirror/CodeMirror/issues/3701#issuecomment-164904534
const syntaxHighlighter = typeof window !== 'undefined' ? require('@readme/syntax-highlighter') : false;
const canonicalLanguage = require('@readme/syntax-highlighter/canonical');
const syntaxHighlighter = typeof window !== 'undefined' ? require('@readme/syntax-highlighter').default : false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now that the server-compatible version of this, index.node.js is the main entrypoint, you might need to reference the client version.

Here's main: https://github.com/readmeio/syntax-highlighter/blob/master/src/index.node.js (no default).
Here's the client: https://github.com/readmeio/syntax-highlighter/blob/master/src/index.js (default)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure what I need to change here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@kellyjosephprice You mind taking this? I'm not sure what exactly this require needs to be changed to.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure!

Copy link
Copy Markdown
Contributor

@kellyjosephprice kellyjosephprice Nov 6, 2020

Choose a reason for hiding this comment

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

I believe there's no change needed? It looks like syntax-highlighter is using the browser field in package.json and the webpack build for markdown is loading the correct file.

$ rg SyntaxHighlighter dist
dist/main.node.js
5057:  }), console.log("SyntaxHighlighter loading server");

dist/main.js
20852:  console.log("SyntaxHighlighter loading client");

Copy link
Copy Markdown
Contributor

@rafegoldberg rafegoldberg left a comment

Choose a reason for hiding this comment

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

Thanks @erunion!

Copy link
Copy Markdown
Contributor

@rafegoldberg rafegoldberg left a comment

Choose a reason for hiding this comment

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

Pulling back here as I just checked this out to verify, and it looks like there are some SSR issues per Gabe's concerns. (To be fair though, I'm using npm link locally so, as always, I'm a touch skeptical as to how well this replicates a "true" install environment...)

[0] webpack:///./node_modules/codemirror/lib/codemirror.js?:17
[0]   var userAgent = navigator.userAgent;
Full Trace
[0] ReferenceError: navigator is not defined
[0]     at eval (webpack:///codemirror/lib/codemirror.js?:17:19)
[0]     at Object../node_modules/codemirror/lib/codemirror.js (/markdown/dist/main.node.js:510:1)
[0]     at __webpack_require__ (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:21:30)
[0]     at eval (webpack:///./node_modules/@readme/syntax-highlighter/codemirror.jsx?:13:18)
[0]     at Object../node_modules/@readme/syntax-highlighter/codemirror.jsx (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:310:1)
[0]     at __webpack_require__ (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:21:30)
[0]     at eval (webpack:///./node_modules/@readme/syntax-highlighter/index.js?:3:18)
[0]     at Object../node_modules/@readme/syntax-highlighter/index.js (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:321:1)
[0]     at __webpack_require__ (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:21:30)
[0]     at eval (webpack:///./components/Code/index.jsx?:12:16)
[0]     at Object../components/Code/index.jsx (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:119:1)
[0]     at __webpack_require__ (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:21:30)
[0]     at eval (webpack:///./components/index.js?:5:63)
[0]     at Module../components/index.js (/Users/rafegoldberg/Repos/ReadMe/markdown/dist/main.node.js:230:1)

I can look a bit closer, unless you want to take this one @kellyjosephprice?

@kellyjosephprice
Copy link
Copy Markdown
Contributor

Yea, looking now.

@rafegoldberg
Copy link
Copy Markdown
Contributor

rafegoldberg commented Nov 7, 2020

So actually, I may have been on an outdated branch when I got that error ‘cause when I run this against next it's working. It does look like these highlighter changes caused some minor problems rendering inline code snippets, though. I think we can fix this by adjusting this line so that we only highlight a code block if it's lang value is explicitly defined?

Update: whelp, that would actually break variable expansion in inline code snippets... The underlying issue is that the updated highlighter now wraps code with a block-level <div> instead of a <span>, which essentially forces a line break in every code snippet. I was able to fix this with a simple CSS solution by inheriting some of the code styles down to the highlight wrapper:

>div[class*="cm-"] {
display: inherit;
}

Before After

Copy link
Copy Markdown
Contributor

@kellyjosephprice kellyjosephprice left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@kellyjosephprice kellyjosephprice merged commit d348025 into next Nov 9, 2020
@kellyjosephprice kellyjosephprice deleted the chore/update-syntaxhighlight branch November 9, 2020 19:54
@rafegoldberg
Copy link
Copy Markdown
Contributor

This PR was released!

🚀 Changes included in v6.22.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants