-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore: convert codebase to ESM #562
Conversation
BREAKING CHANGE: JavaScript API is now ESM only and no longer supports CJS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, ESM 🔥 one thing I was wondering about, should we update all the outdated dependencies as well while doing this breaking change?
import scrollIntoView from 'scroll-into-view-if-needed'; | ||
|
||
const main = () => { | ||
// eslint-disable-next-line no-undef | ||
const node = document.getElementById('hero'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that this is the part under test, but I think this missing line is what eslint
is complaining about 😅
+1, would be nice to do a sweep of the open PRs and bump deps that would be a breaking change. |
Might be worth either merging this into a "next" and then updating dept, then merging into main or doing dep updates as part of this PR to avoid multiple breaking changes |
This PR is now against next and I'll merge it into that and continue work updating dependencies from there |
BREAKING CHANGE: JavaScript API is now ESM only and no longer supports CJS