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

Not working with ESM #61

Closed
ybiquitous opened this issue Aug 14, 2021 · 24 comments · Fixed by #62
Closed

Not working with ESM #61

ybiquitous opened this issue Aug 14, 2021 · 24 comments · Fixed by #62

Comments

@ybiquitous
Copy link

ybiquitous commented Aug 14, 2021

Hi! Thank you for providing this nice preset!

I found a behavior that this preset is not work with ESM. It occurs with the latest version remark-preset-prettier@0.5.1.

Let me show for the details.

Reproduction

Prepare the following files.

package.json:

{
  "type": "module",
  "dependencies": {
    "remark-cli": "10.0.0",
    "remark-preset-lint-markdown-style-guide": "5.0.0",
    "remark-preset-prettier": "0.5.1"
  },
  "scripts": {
	"remark": "remark ."
  }
}

.remarkrc.js:

export default {
  plugins: [
    'remark-preset-lint-markdown-style-guide',
    'remark-preset-prettier',
  ],
};

test.md:

-   a
-   b

Run:

$ npm run remark

> remark
> remark .

test.md
  1:5  warning  Incorrect list-item indent: remove 2 spaces  list-item-indent  remark-lint
  2:5  warning  Incorrect list-item indent: remove 2 spaces  list-item-indent  remark-lint

⚠ 2 warnings

I expect no warnings, but 2 warnings are detected.

When I inspected node_modules/remark-preset-prettier/lib/index.js, I found an ignored exception (_a) at line 43:

    try {
        plugins.push([cjsRequire('remark-lint-' + plugin), false]);
    }
    catch (_a) { }
    return plugins;

This ignored exception's message is something like:

Error [ERR_REQUIRE_ESM]: require() of ES Module /private/tmp/foo/node_modules/remark-lint-unordered-list-marker-style/index.js from /private/tmp/foo/node_modules/remark-preset-prettier/lib/index.js not supported.
Instead change the require of /private/tmp/foo/node_modules/remark-lint-unordered-list-marker-style/index.js in /private/tmp/foo/node_modules/remark-preset-prettier/lib/index.js to a dynamic import() which is available in all CommonJS modules.

Maybe, this exception is related to this behavior. 🤔

Environment

  • OS: macOS 11.5.2 20G95
  • Node.js: 16.6.1
  • npm: 7.20.3

If the reproduction above or my understandings are incorrect, please feel free to tell me or close this issue.
Thank you.

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

Thank you, I'll take a look soon.

So remark-lint-list-item-indent is esm only now.

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

@wooorm Hi, I need help here. Is that possible to use await import() in a remark preset? Or do I have to list all possible deps in package.json and use import statement instead?

JounQin added a commit that referenced this issue Aug 14, 2021
@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

@ybiquitous Please try the following version for testing:

yarn add https://pkg.csb.dev/JounQin/remark-preset-prettier/commit/45ec5d24/remark-preset-prettier
npm i https://pkg.csb.dev/JounQin/remark-preset-prettier/commit/45ec5d24/remark-preset-prettier

Although I don't think this is the perfect solution because it may install unexpected remark-lint plugins for end users.

JounQin added a commit that referenced this issue Aug 14, 2021
JounQin added a commit that referenced this issue Aug 14, 2021
@wooorm
Copy link

wooorm commented Aug 14, 2021

Is that possible to use await import() in a remark preset?

No, the configuration stage is sync. This is a similar issue to: remarkjs/remark-retext#14.

Or do I have to list all possible deps in package.json and use import statement instead?

Yes. But: you could use createRequire from node:module if you only care about Node: https://github.com/remarkjs/remark/blob/b767382a29994200ca2ae3cc4ad9ecf36decf91f/packages/remark-cli/cli.js#L2.

But I think importing the plugins you want to turn off might be best

@ybiquitous
Copy link
Author

Please try the following version for testing:

@JounQin Thanks. The version works expectedly! 👍🏼

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

@wooorm

Yes. But: you could use createRequire from node:module if you only care about Node: remarkjs/remark@b767382/packages/remark-cli/cli.js#L2.

I'm using createRequire indeed: https://github.com/JounQin/remark-preset-prettier/blob/master/index.ts#L3-L4

But cjsRequire will just fail on native esm plugin as this issue described because remark-lint-list-item-indent is native esm only:

Error [ERR_REQUIRE_ESM]: require() of ES Module /private/tmp/foo/node_modules/remark-lint-unordered-list-marker-style/index.js from /private/tmp/foo/node_modules/remark-preset-prettier/lib/index.js not supported.
Instead change the require of /private/tmp/foo/node_modules/remark-lint-unordered-list-marker-style/index.js in /private/tmp/foo/node_modules/remark-preset-prettier/lib/index.js to a dynamic import() which is available in all CommonJS modules.

But I think importing the plugins you want to turn off might be best

As I say:

it may install unexpected remark-lint plugins for end users.

For end users not using latest remark-lint plugins, that would be broken. What means it can not be compatible any more.

@wooorm
Copy link

wooorm commented Aug 14, 2021

For end users not using latest remark-lint plugins, that would be broken. What means it can not be compatible any more.

Yes. Updating dependencies and publishing a major version would solve that though?

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

Yes. Updating dependencies and publishing a major version would solve that though?

Well, if there is no better way to fix this, I have to do like that. 😂

Although I still hope there will be better solution like remarkjs/remark-retext#14 or supporting await import() in preset.

@wooorm
Copy link

wooorm commented Aug 14, 2021

Well, top-level await is supported in Node 14 and 16?

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

@ybiquitous I've just released remark-preset-prettier@v1.0.0.

@ybiquitous
Copy link
Author

ybiquitous commented Aug 14, 2021

@JounQin Thank you so much for the quick fix! I can confirm the new version 1.0.0 works well. 🥰

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

@wooorm As https://github.com/stylelint/remark-preset/pull/6/files#diff-4cfcf9277f6a14080f988dbe6158fedb46b1049c56bc815e42a2736e0b215c70R6, I found [pluginStr, false] would still work, and because of false, even if pluginStr is actually non-existed, it will still work!

export default {
  plugins: [['non-existed-dep', false]],
}

Is this valid? If so, I can still remove all dependencies.

@wooorm
Copy link

wooorm commented Aug 14, 2021

Strings are supported in configuration files of the unified engine (so in .remarkrc.js), but not in presets like remark-preset-prettier.
It is valid in a .remarkrc but it’s not valid in a preset

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

@wooorm

Strings are supported in configuration files of the unified engine (so in .remarkrc.js), but not in presets like remark-preset-prettier.
It is valid in a .remarkrc but it’s not valid in a preset

Do you mean @ybiquitous's changes https://github.com/stylelint/remark-preset/pull/6/files#diff-4cfcf9277f6a14080f988dbe6158fedb46b1049c56bc815e42a2736e0b215c70R6 is incorrect actually?

But I tried like following and it does work:

export default {
  plugins: [
    'preset-lint-consistent',
    'preset-lint-markdown-style-guide',
    'preset-lint-recommended',
    './lib/index.js',
  ],
}

Is there anything I missed?

@wooorm
Copy link

wooorm commented Aug 14, 2021

No, that change is good. Because it’s in a .remarkrc file.

Your change is not good, as it is not in a .remarkrc file.

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

@wooorm
Copy link

wooorm commented Aug 14, 2021

Then that change is incorrect: it does not work

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

Then that change is incorrect: it does not work

cc @ybiquitous


@wooorm

I have an idea to create a package named remark-config-prettier to be used in remarkrc only.

Maybe @stylelint/remark-preset is only used in that way too, the preset is confusing then.

Will string config contains string plugins in remarkrc work?

{
  "plugins": ["@stylelint/remark-preset"]
}

@wooorm
Copy link

wooorm commented Aug 14, 2021

Only one rc file is allowed. They can be shared (put in node_modules/).

However, I strongly recommend against that. Presets work on the API. Can be bundled in a browser. Config files and strings don’t
This conversation is getting close to: remarkjs/remark-retext#14.

Will string config contains string plugins in remarkrc work?

That works in an rc file. It does not work in a preset

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

Only one rc file is allowed. They can be shared (put in node_modules/).

That works in an rc file. It does not work in a preset

I'm still not sure to understand, I mean if @stylelint/remark-preset changes like that PR, will a .remarkrc in another project work?

// .remarkrc
{
  "plugins": ["@stylelint/remark-preset"]
}

However, I strongly recommend against that. Presets work on the API. Can be bundled in a browser. Config files and strings don’t

I understand that, I'm talking another package which maybe only aim to remarkrc config on Node.js. remark-preset-prettier will not change like that.

@wooorm
Copy link

wooorm commented Aug 14, 2021

No, I don’t think it will work. That PR is I believe broken.

I understand that, I'm talking another package which maybe only aim to remarkrc config on Node.js. remark-preset-prettier will not change like that.

I’m not sure how it could be used. It could not be combined with other presets. It just turns stuff off. So why not use no rc file at all?

@JounQin
Copy link
Member

JounQin commented Aug 14, 2021

No, I don’t think it will work. That PR is I believe broken.

OK, if it does not work, remark-config-prettier I proposed is meaningless too maybe.

I was thinking the usage:

{
  "plugins": [
    // ... othere presets
    "config-prettier" // It just turns stuff off for prettier.
  ]
}

@ybiquitous
Copy link
Author

@JounQin @wooorm Thank you so much for your help! 🥰

@ybiquitous
Copy link
Author

@JounQin @wooorm Thanks again! We were able to release @stylelint/remark-preset@3.0.0 successfully. 😄

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 a pull request may close this issue.

3 participants