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.

Sign up for GitHub

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

Jump to bottom

feat(spectral): migrate to ESM #718

Merged
merged 6 commits into from
Aug 3, 2023
Merged

feat(spectral): migrate to ESM #718

merged 6 commits into from
Aug 3, 2023

Conversation

kanadgupta
Copy link
Member

@kanadgupta kanadgupta commented Aug 3, 2023 •
edited
Loading

🧰 Changes

Figured I'd use the spectral-config package as an exercise in migrating packages to ESM 🙃 there's a few reasons behind doing this:

  • We can't use the latest version of alex otherwise, which is causing some headaches in https://github.com/readmeio/readme/pull/9724
  • Since we're loading this package into the Spectral1 and not anywhere in our actual codebase, this shouldn't be a breaking change where we use it (though we should probably consider this a breaking change since it now requires Node 16+).

🧬 QA & Testing

Tests still pass, but I'm not sure how to hide all those ExperimentalWarning logs. (EDIT: never mind, they don't show up when running tests via lerna!)

Footnotes

  1. Spectral supports functions defined via ESM as of 6.x, see the bottom of this page: https://meta.stoplight.io/docs/spectral/a781e290eb9f9-custom-functions#code-transpilation (Spotlight's hash links don't work properly, go figure) ↩

@kanadgupta kanadgupta added the enhancement New feature or request label Aug 3, 2023
@kanadgupta kanadgupta requested a review from erunion August 3, 2023 15:40
@kanadgupta kanadgupta marked this pull request as ready for review August 3, 2023 15:40
erunion
erunion reviewed Aug 3, 2023
View reviewed changes
Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

lgtm. mind also expanding CI to also run everything against node 20? and maybe drop 19 since it's EOLd

@kanadgupta
Copy link
Member Author

lmao oops accidentally committed it to main: bccb825

@erunion erunion merged commit b1a4ee5 into main Aug 3, 2023
3 checks passed
@erunion erunion deleted the spectral-esm branch August 3, 2023 22:22
kanadgupta added a commit that referenced this pull request Aug 11, 2023
@kanadgupta
Revert "feat(spectral): migrate to ESM (#718)"
8b3669e 
This reverts commit b1a4ee5.
@kanadgupta kanadgupta mentioned this pull request Aug 11, 2023
Merged
erunion pushed a commit that referenced this pull request Aug 14, 2023
@kanadgupta
fix(spectral-config): partially revert ESM (#724)
c4161e3 
* chore: more prettier cleanup

* Revert "feat(spectral): migrate to ESM (#718)"

This reverts commit b1a4ee5.

* revert: undo dependabot changes

* chore: revert dep changes

* chore: specify engines

* chore: hide test warnings

* fix: dynamically import alex

* chore: lint
This was referenced Sep 8, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erunion erunion erunion approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kanadgupta @erunion