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

Migrating to ECMAScript modules #894

Merged
merged 28 commits into from
Aug 21, 2023
Merged

Migrating to ECMAScript modules #894

merged 28 commits into from
Aug 21, 2023

Conversation

Janther
Copy link
Contributor

@Janther Janther commented Jul 25, 2023

I know this PR looks very daunting.

Here are the main changes:

  • elsint, jest, and webpack configurations were changed accordingly
  • the build target file is now dist/standalone.cjs instead of dist/standalone.js
  • all require were switched to import
  • all module.exports were switched to export default or export individual functions
  • all __dirname were changed to meta.import and then extracted from that value
  • some functions used the variable doc that was changed to document since now the import exposes prettier.doc directly
  • the generateindexes script had to be removed since dir-to-object doesn't support esm
  • the testing configuration scripts were adapted based on the changes that prettier made to their tests when they migrated
  • standalone tests use a commonjs environment and normal tests use an esm environment so we need esmock or proxyquire depending on the situation

Caveats:

  • if you are still using prettier 2 or a commonjs development environment, then you have to use the compiled version of the plugin dist/standalone.cjs (this is done automatically by node and conditional exports), unit tests for prettier 2 had to be disabled for this reason

@Janther Janther requested review from fvictorio and mattiaerre July 25, 2023 19:49
@fvictorio
Copy link
Member

Hey @Janther, I won't have time to review this but I did try it locally with both Prettier v2 and v3 (using verdaccio, which I recommend for scenarios like this one).

Prettier v3 works fine, but v2 doesn't because the dist/standalone.cjs file wasn't included in the files array. I think the easiest thing to do is to just include the full directory, which I did in the last commit. But if you prefer explicitly listing each file, feel free to revert that commit and use that approach.

In any case, I'm pre-approving it.

"dist/standalone.js",
"dist/standalone.js.LICENSE.txt",
"dist/standalone.js.map",
"dist/**/*",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad. I'll push a commit making sure webpack cleans the folder before building to avoid publishing random stuff

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.

2 participants