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

chore: allow cjs and mjs info strings #176

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Feb 23, 2021

It seems ESLint folks are not interested in my meta string proposal, I guess the right move would be to move from

```js esm
// ES module
export {};
```

```js cjs
// CJS module
module.exports = {};
```

```js
// Whichever
console.log();
```

to

```mjs
// ES module
export {};
```

```cjs
// CJS module
module.exports = {};
```

```js
// Whichever
console.log();
```

Refs: nodejs/node#37162
Refs: nodejs/node#37311
Refs: eslint/markdown#170


FWIW, I'd be up to fork the eslint markdown plugin and maintain ourselves support for the meta string instead, but maybe it's not worth the maintenance burden.

Copy link
Member

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

I'm assuming this works with HighlightJS/Prism and that maybe there is a downstream need to update those CSS/JS bundles to support this when building the docs in nodejs/node

@nschonni
Copy link
Member

This was the other "metadata" style I was trying to think of before https://prettier.io/blog/2020/11/20/2.2.0.html#align-code-block-language-detection-with-other-popular-tools-9365httpsgithubcomprettierprettierpull9365-by-kachkaevhttpsgithubcomkachkaev

@aduh95
Copy link
Contributor Author

aduh95 commented Feb 23, 2021

I'm assuming this works with HighlightJS

Yes, it works with highlight.js (not sure for Prism), so no need to change the nodejs/node doc generation tooling.

@aduh95
Copy link
Contributor Author

aduh95 commented Mar 1, 2021

@nodejs/linting Should we land this?

@aduh95
Copy link
Contributor Author

aduh95 commented Mar 4, 2021

@Trott Any chance you'll have time to publish this to npm? I don't think I have the permission to do it myself.

@Trott Trott merged commit 5a93237 into nodejs:master Mar 4, 2021
@Trott
Copy link
Member

Trott commented Mar 4, 2021

Published 2.1.1 which includes this.

@aduh95 aduh95 deleted the doc-multi-syntax-snippets branch March 4, 2021 20:32
aduh95 added a commit to aduh95/node that referenced this pull request Mar 4, 2021
aduh95 added a commit to aduh95/node that referenced this pull request Mar 4, 2021
aduh95 added a commit to aduh95/node that referenced this pull request Mar 4, 2021
aduh95 added a commit to aduh95/node that referenced this pull request Mar 4, 2021
Trott pushed a commit to aduh95/node that referenced this pull request Mar 6, 2021
Refs: nodejs/remark-preset-lint-node#176

PR-URL: nodejs#37605
Refs: nodejs#37162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
aduh95 added a commit to aduh95/node that referenced this pull request Mar 6, 2021
danielleadams pushed a commit to nodejs/node that referenced this pull request Mar 16, 2021
Refs: nodejs/remark-preset-lint-node#176

PR-URL: #37605
Refs: #37162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit to nodejs/node that referenced this pull request Mar 16, 2021
targos pushed a commit to nodejs/node that referenced this pull request May 30, 2021
targos pushed a commit to nodejs/node that referenced this pull request Jun 5, 2021
targos pushed a commit to nodejs/node that referenced this pull request Jun 5, 2021
targos pushed a commit to nodejs/node that referenced this pull request Jun 11, 2021
targos pushed a commit to targos/node that referenced this pull request Aug 8, 2021
Refs: nodejs/remark-preset-lint-node#176

PR-URL: nodejs#37605
Refs: nodejs#37162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Aug 13, 2021
Refs: nodejs/remark-preset-lint-node#176

PR-URL: nodejs#37605
Refs: nodejs#37162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Sep 1, 2021
Refs: nodejs/remark-preset-lint-node#176

PR-URL: nodejs#37605
Refs: nodejs#37162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit to nodejs/node that referenced this pull request Sep 1, 2021
Refs: nodejs/remark-preset-lint-node#176

PR-URL: #37605
Refs: #37162
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
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