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

fix(esm): fix esm/commonjs compatibility issues with custom formatters/snippets #1903

Merged
merged 10 commits into from
Feb 20, 2022

Conversation

davidjgoss
Copy link
Contributor

@davidjgoss davidjgoss commented Jan 29, 2022

Description

Handle all permutations of exporting a custom formatter or snippet class from userland code, across ESM and CommonJS.

It's ugly but it works and has solid tests, and when we get to a point (probably in another 1-2 major releases) when we can go ESM only then most of the complexity can be removed.

Also, add a couple of missing things to the main export.

Motivation & context

To prevent regression from current functionality as we switch from require to import. This was affecting the pretty formatter with the latest RC.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • My change needed additional tests
    • I have added tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

@quine-bot
Copy link

quine-bot bot commented Jan 29, 2022

👋 Figuring out if a PR is useful is hard, hopefully this will help.

  • @davidjgoss has been on GitHub since 2013 and in that time has had 165 public PRs merged
  • Don't you recognize them? They've been here before 🎉
  • Here's a good example of their work: liquibase-linter (Quality control for your Liquibase scripts)
  • From looking at their profile, they seem to be good with JavaScript and TypeScript.

Their most recently public accepted PR is: #1895

@coveralls
Copy link

coveralls commented Jan 29, 2022

Coverage Status

Coverage increased (+0.004%) to 98.168% when pulling e7023e0 on fix/deep-imports-and-esm into 3757f3c on main.

@davidjgoss davidjgoss changed the title fix: slim down exports config to resolve import/require issues fix(esm): fix esm/commonjs compatibility issues Jan 29, 2022
@davidjgoss davidjgoss changed the title fix(esm): fix esm/commonjs compatibility issues fix(esm): fix esm/commonjs compatibility issues with custom formatters/snippets Feb 17, 2022
@davidjgoss davidjgoss added this to the ESM milestone Feb 17, 2022
@davidjgoss davidjgoss marked this pull request as ready for review February 17, 2022 16:07
@davidjgoss davidjgoss merged commit ee54681 into main Feb 20, 2022
@davidjgoss davidjgoss deleted the fix/deep-imports-and-esm branch February 20, 2022 10:45
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