🐛 Rollback sdk-utils to commonjs for broader compatibility#916
Merged
🐛 Rollback sdk-utils to commonjs for broader compatibility#916
Conversation
Revive babel-register.cjs and babel-plugin-module-resolver to recreate the same behavior that our custom ESM loader achieves. The default preset-env auto handling only determines ESM support based on the root package. Since we allow packages to share files without building, babel needs to be configured in way so that individual files have their ES modules preserved when necessary. This is done by utilizing babel overrides and a filter which checks cached package info for files.
While these dynamic imports will not be encountered in browser bundles, some external bundlers may attempt to resolve theses dynamic imports anyway, or may fail to include the bundle entirely.
Some external build tools and bundlers are still not ready to handle ESM packages and all the features that come with. Reverting this package back to commonjs provides the best support since ESM packages will still be able to import commonjs packages.
Not only does this reduce dependencies of sdk-utils, but also fixes issues with bundlers that do not properly support ESM import syntax used within @percy/logger, and additionally results in less dependency conflicts when newer versions of logger are included with the CLI.
Remote browser support within logger exists so that sdk-utils' included logger can successfully connect with remote versions of itself. However, now that sdk-utils uses its own mini logger with remote browser support, the main logger class no longer needs to include these features.
The root endpoint is still supported for backwards compatibility
Contributor
|
Will resolve #869 |
ebdc725 to
942af0f
Compare
942af0f to
a60f2a0
Compare
Robdel12
approved these changes
May 4, 2022
Contributor
Robdel12
left a comment
There was a problem hiding this comment.
🏁 Love it, this will solve a little of little annoying issues that proliferated from ESM.
| "test/helpers.js" | ||
| ], | ||
| "main": "./dist/index.js", | ||
| "browser": "./dist/bundle.js", |
And use Error.prototype.toString to parse serialized errors
This was referenced May 9, 2022
Closed
Robdel12
added a commit
to percy/percy-playwright
that referenced
this pull request
May 9, 2022
We converted `@percy/sdk-utils` to be ESM for 1.x, and this causes a lot of various issues. That has since been reverted (it's back to commonjs with percy/cli#916) so we can remove the dynamic import to fully remove any issues with ESM.
Robdel12
added a commit
to percy/percy-playwright
that referenced
this pull request
May 9, 2022
We converted `@percy/sdk-utils` to be ESM for 1.x, and this causes a lot of various issues. That has since been reverted (it's back to commonjs with percy/cli#916) so we can remove the dynamic import to fully remove any issues with ESM.
kodiakhq bot
referenced
this pull request
in carbon-design-system/carbon-for-ibm-dotcom
May 24, 2022
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@percy/cli](https://togithub.com/percy/cli) | [`~1.1.0` -> `~1.2.0`](https://renovatebot.com/diffs/npm/@percy%2fcli/1.1.0/1.2.1) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [@percy/dom](https://togithub.com/percy/cli) | [`~1.1.0` -> `~1.2.0`](https://renovatebot.com/diffs/npm/@percy%2fdom/1.1.0/1.2.1) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [@percy/sdk-utils](https://togithub.com/percy/cli) | [`~1.1.0` -> `~1.2.0`](https://renovatebot.com/diffs/npm/@percy%2fsdk-utils/1.1.0/1.2.1) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>percy/cli</summary> ### [`v1.2.1`](https://togithub.com/percy/cli/releases/tag/v1.2.1) [Compare Source](https://togithub.com/percy/cli/compare/v1.2.0...v1.2.1) <!-- Release notes generated using configuration in .github/release.yml at master --> #### What's Changed ##### 🐛 Bug Fixes - 🐛 Fix sitemap xml parsing regex (stop when the lookahead matches) by [@​Robdel12](https://togithub.com/Robdel12) in [https://github.com/percy/cli/pull/928](https://togithub.com/percy/cli/pull/928) **Full Changelog**: percy/cli@v1.2.0...v1.2.1 ### [`v1.2.0`](https://togithub.com/percy/cli/releases/tag/v1.2.0) [Compare Source](https://togithub.com/percy/cli/compare/v1.1.4...v1.2.0) <!-- Release notes generated using configuration in .github/release.yml at master --> #### What's Changed ##### ✨ Enhancements - ✨ Add additional snapshot command exec helpers by [@​wwilsman](https://togithub.com/wwilsman) in [https://github.com/percy/cli/pull/922](https://togithub.com/percy/cli/pull/922) ##### 🐛 Bug Fixes - 🐛 Fix regex matching accepting non-regex patterns by [@​wwilsman](https://togithub.com/wwilsman) in [https://github.com/percy/cli/pull/923](https://togithub.com/percy/cli/pull/923) ##### ⬆️⬇️ Dependency Updates - ⬆️ Bump karma from 6.3.19 to 6.3.20 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/927](https://togithub.com/percy/cli/pull/927) - ⬆️ Bump yaml from 2.0.1 to 2.1.0 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/925](https://togithub.com/percy/cli/pull/925) - ⬆️ Bump karma-jasmine from 5.0.0 to 5.0.1 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/926](https://togithub.com/percy/cli/pull/926) **Full Changelog**: percy/cli@v1.1.4...v1.2.0 ### [`v1.1.4`](https://togithub.com/percy/cli/releases/tag/v1.1.4) [Compare Source](https://togithub.com/percy/cli/compare/v1.1.3...v1.1.4) <!-- Release notes generated using configuration in .github/release.yml at master --> #### What's Changed ##### 🐛 Bug Fixes - 🐛 Request fonts as binary via Node by [@​Robdel12](https://togithub.com/Robdel12) in [https://github.com/percy/cli/pull/921](https://togithub.com/percy/cli/pull/921) **Full Changelog**: percy/cli@v1.1.3...v1.1.4 ### [`v1.1.3`](https://togithub.com/percy/cli/releases/tag/v1.1.3) [Compare Source](https://togithub.com/percy/cli/compare/v1.1.2...v1.1.3) <!-- Release notes generated using configuration in .github/release.yml at master --> #### What's Changed ##### 🐛 Bug Fixes - 🐛 Fix sdk-utils test helper log capture by [@​wwilsman](https://togithub.com/wwilsman) in [https://github.com/percy/cli/pull/920](https://togithub.com/percy/cli/pull/920) ##### ⬆️⬇️ Dependency Updates - ⬆️ Bump path-to-regexp from 6.2.0 to 6.2.1 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/919](https://togithub.com/percy/cli/pull/919) - ⬆️ Bump rollup from 2.71.1 to 2.72.1 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/918](https://togithub.com/percy/cli/pull/918) **Full Changelog**: percy/cli@v1.1.2...v1.1.3 ### [`v1.1.2`](https://togithub.com/percy/cli/releases/tag/v1.1.2) [Compare Source](https://togithub.com/percy/cli/compare/v1.1.1...v1.1.2) <!-- Release notes generated using configuration in .github/release.yml at master --> #### What's Changed ##### 🐛 Bug Fixes - 🐛 Fix logger TTY logic for logging progress by [@​wwilsman](https://togithub.com/wwilsman) in [https://github.com/percy/cli/pull/917](https://togithub.com/percy/cli/pull/917) **Full Changelog**: percy/cli@v1.1.1...v1.1.2 ### [`v1.1.1`](https://togithub.com/percy/cli/releases/tag/v1.1.1) [Compare Source](https://togithub.com/percy/cli/compare/v1.1.0...v1.1.1) <!-- Release notes generated using configuration in .github/release.yml at master --> #### What's Changed ##### 🐛 Bug Fixes - 🐛 Rollback sdk-utils to commonjs for broader compatibility by [@​wwilsman](https://togithub.com/wwilsman) in [https://github.com/percy/cli/pull/916](https://togithub.com/percy/cli/pull/916) ##### ⬆️⬇️ Dependency Updates - ⬆️ Bump karma from 6.3.18 to 6.3.19 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/900](https://togithub.com/percy/cli/pull/900) - ⬆️ Bump [@​babel/preset-env](https://togithub.com/babel/preset-env) from 7.16.11 to 7.17.10 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/913](https://togithub.com/percy/cli/pull/913) - ⬆️ Bump rollup from 2.70.2 to 2.71.1 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/912](https://togithub.com/percy/cli/pull/912) - ⬆️ Bump [@​babel/cli](https://togithub.com/babel/cli) from 7.17.6 to 7.17.10 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/911](https://togithub.com/percy/cli/pull/911) - ⬆️ Bump ws from 8.5.0 to 8.6.0 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/914](https://togithub.com/percy/cli/pull/914) - ⬆️ Bump [@​babel/core](https://togithub.com/babel/core) from 7.17.9 to 7.17.10 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/910](https://togithub.com/percy/cli/pull/910) - ⬆️ Bump [@​rollup/plugin-node-resolve](https://togithub.com/rollup/plugin-node-resolve) from 13.2.1 to 13.3.0 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/percy/cli/pull/915](https://togithub.com/percy/cli/pull/915) **Full Changelog**: percy/cli@v1.1.0...v1.1.1 </details> --- ### Configuration 📅 **Schedule**: "every weekend" (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Never, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/carbon-design-system/carbon-for-ibm-dotcom).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this?
With Node 14 being the oldest currently supported LTS release, ES modules and accompanying features should be supported by the JavaScript ecosystem, as long as you are on a maintained version of Node. However, it seems that this notion is a fallacy, as several JavaScript tools come with their own build systems and bundlers, which themselves may not have parity with Node's LTS releases.
For this reason, the
@percy/sdk-utilspackage, which is used by most other JavaScript based SDKs, needs to be rolled back to being a commonjs module for the time being. Some babel packages and files were revived from the graveyard to recreate the same behavior that our custom ESM loader achieves. Since we allow packages to share files without building, babel needs to be configured in way so that individual files have their ES modules preserved when necessary. This is done by utilizing babel overrides and a filter which checks cached package info for files.Unfortunately, a couple of other changes also needed to be made to make
@percy/sdk-utilsplay nicely with third-party build tools and bundlers:First, the Rollup bundles will now have their dynamic imports scrubbed. While the codepath to these imports isn't ever actually taken, build tools and bundlers will sometimes attempt to build and bundle those imports anyway. The Rollup bundles now replace
import(.*)with({})to act as a no-op.Second,
@percy/sdk-utilswill no longer include@percy/loggerdirectly. Instead, it contains it's own cross-platform mini-logger still capable of remote logging connections. This change allows the logger to continue to remain an ES module without it needing to worry about where sdk-utils gets bundled.Because of the last change,
@percy/loggerno longer needs to support browsers or remote connections itself since these features only existed for sdk-utils. The package has been refactored to remove these features, and the remaining remote code for accepting connections was moved into core's API WebSocket server. While doing so, I also added a logger specific endpoint for remote connections while maintaining compatibility with root endpoint connections.