Skip to content

chore: Rollup & Vitest#1083

Closed
duncanbeevers wants to merge 3 commits intoprettier:masterfrom
duncanbeevers:rollup-and-vitest
Closed

chore: Rollup & Vitest#1083
duncanbeevers wants to merge 3 commits intoprettier:masterfrom
duncanbeevers:rollup-and-vitest

Conversation

@duncanbeevers
Copy link
Copy Markdown

🗣️ Discussion

This PR is intended to get prettier-eslint running on a little more modern testing infrastructure, and to emit CJS and ESM bundles.
Specifically, I'd like to add support for handling newer TypeScript versions and flat configs (re; #947, #1015, #1036,)

🔧 Changes

  • Use Rollup instead of Babel to generate CJS and ESM bundles
  • Convert implementation to ESM
  • Use Vitest instead of Jest for tests
  • Use memfs instead of mocking specific node:fs functions
  • Update nps calls to use CJS config

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Dec 7, 2024

🦋 Changeset detected

Latest commit: 785dd34

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
prettier-eslint Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@phun-ky
Copy link
Copy Markdown
Contributor

phun-ky commented Jan 29, 2025

@duncanbeevers can we join forces on this #1036, so we can support eslint@9? and @JounQin , could you reopen the PR or will it do it automagically when we push new changes?

@duncanbeevers
Copy link
Copy Markdown
Author

@phun-ky Feel free to port any / all of this over to your PR. I used #1036 as a launching-point for this work, so hopefully it shouldn't be too difficult to reconcile the two.

I'm happy close this PR, or to leave it open until the other PR is resuscitated. Let me know how I can help!

@JounQin
Copy link
Copy Markdown
Member

JounQin commented Jan 30, 2025

Great job, and I've reopened #1036

package.json Outdated
@@ -52,20 +54,29 @@
"@babel/preset-env": "^7.22.9",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These dependencies can be dropped.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Are you certain? The options we pass to prettier include { parser: 'babel' }.
If prettier ships with its own babel parser stuff I guess that's fine.

I pushed the PR with the babel dependencies removed, and the tests are passing, so hopefully everything is peachy.

* Use Rollup instead of Babel to generate CJS and ESM bundles
* Convert implementation to ESM
* Use Vitest instead of Jest for tests
* Use memfs instead of mocking specific node:fs functions
* Update nps calls to use CJS config
@phun-ky
Copy link
Copy Markdown
Contributor

phun-ky commented Feb 6, 2025

@duncanbeevers do not merge this PR, have ported to #1036

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2025

Stale pull request

Comment on lines +386 to +388
return await import(modulePath).then(
({ default: defaultExport }) => defaultExport,
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function should not intercept the default export by default.

eslint/prettier both support named exports.

nodeResolve({ preferBuiltins: true }),
json(),
],
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Personally, I prefer to use esbuild.

logger.trace(`requiring "${name}" module at "${modulePath}"`);
return require(modulePath);
logger.trace(`importing "${name}" module at "${modulePath}"`);
return await import(modulePath).then(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This import() will fail on Windows, since it doesn't take Windows path.

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.

4 participants