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

Unable to bundle stylelint using bundlers #6440

Closed
phoenisx opened this issue Nov 1, 2022 · 5 comments · Fixed by #6449
Closed

Unable to bundle stylelint using bundlers #6440

phoenisx opened this issue Nov 1, 2022 · 5 comments · Fixed by #6449
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule

Comments

@phoenisx
Copy link
Contributor

phoenisx commented Nov 1, 2022

What steps are needed to reproduce the bug?

Use rollup to bundle this library. It fails

What Stylelint configuration is needed to reproduce the bug?

None

How did you run Stylelint?

I am trying to bundle stylelint in my project

Which version of Stylelint are you using?

14.14.0

What did you expect to happen?

Stylelint should get bundled, without any errors.

What actually happened?

Getting Could not dynamically require "./color-no-invalid-hex" when using color-no-invalid-hex in the project.

Does the bug relate to non-standard syntax?

None

Proposal to fix the bug

Since the latter does not use the import-lazy pattern for bundlers, bundlers are not able to resolve the require paths and fails during runtime.

Possible Resolution:

  • We can keep the import-lazy pattern for bundlers intact, i.e. to revert back to using importLazy(() => require('./color-no-invalid-hex'))() pattern
  • To fix usages of // @ts-nocheck, we can do something like following instead:
/** @type {(fn: () => any) => (() => any)} */
// @ts-ignore
const importLazy = require('import-lazy');
...
  'color-no-invalid-hex': importLazy(() => require('./color-no-invalid-hex'))(),

I am not sure how strict this project is in terms of using @ts-ignore comments, but I feel this is a decent trade-off to support proper bundling, unless someone has a better solution to overcome typescript type errors in rules module.

@ybiquitous
Copy link
Member

@phoenisx Thanks for opening the issue and for using the template.

Your suggestion sounds good to me. Could you please provide a minimal reproduction repository using rollup?

@ybiquitous ybiquitous added the status: needs reproducible example triage needs reproducible demo or repo label Nov 1, 2022
@phoenisx
Copy link
Contributor Author

phoenisx commented Nov 1, 2022

Hi @ybiquitous
Thanks for the quick reply 🙇🏽

I created a repro for this issue here: https://github.com/phoenisx/stylelint-bundling-issue

You need to run the following commands to get to the reproduction:

pnpm build #use `npm` if you prefer
pnpm run-bundle

You will get the following error:

Failed to process:  Error: Could not dynamically require "./color-no-invalid-hex".
Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option
of @rollup/plugin-commonjs appropriately for this require call to work.

I have also tried adding dynamicRequireTargets, but with no success.

@ybiquitous
Copy link
Member

@phoenisx Thank you for providing the reproduction with a detailed explanation! Converting the code to the following bundler-friendly pattern seems to make sense. 👍🏼

-	'alpha-value-notation': importLazy('./alpha-value-notation'),
+	'alpha-value-notation': importLazy(() => require('./alpha-value-notation'))(),

In addition, to avoid type errors, we need to apply the patch to the import-lazy type definition like this:

 declare function importLazy<T = unknown>(
 	importFn: (moduleId: string) => T
-): (moduleId: string) => T;
+): (moduleId?: string) => T;

Let's create and commit a patch by using the patch-package package.


I've labeled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

@ybiquitous ybiquitous added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule and removed status: needs reproducible example triage needs reproducible demo or repo labels Nov 2, 2022
@phoenisx
Copy link
Contributor Author

phoenisx commented Nov 2, 2022

Thanks for the guidance. I can work on this and raise a fix later.

@phoenisx
Copy link
Contributor Author

phoenisx commented Nov 3, 2022

I have raised the PR. Do let me know if anything else is needed to resolve this issue 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

2 participants