-
Notifications
You must be signed in to change notification settings - Fork 3
[LOOM-2186]: Convert to strict es modules #146
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
Conversation
@@ -13,7 +13,7 @@ | |||
limitations under the License. | |||
*/ | |||
|
|||
import useTypographyStylesPlugin from './lib/rules/use-typography-styles'; | |||
import useTypographyStylesPlugin from './lib/rules/use-typography-styles/index.js'; |
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.
A small question:
Since the "import/extensions": "off"`, why do we add the file suffix? 🤔 If I'm not mistaken, when this config is off, it supports both suffixed and unsuffixed files. Are there any special considerations here? Or I missed something?
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.
Pull Request Overview
This PR updates the package to strict ES modules syntax, adds explicit exports in package.json, and adjusts import paths and linting to support ESM.
- Add
exports
field inpackage.json
for ESM entry points - Migrate CommonJS
module.exports
toexport
syntax in utility modules - Update import statements to use
.js
extensions across rules and root index - Adjust ESLint config to accommodate extended import patterns
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
package.json | Defined ESM exports to support strict module resolution |
lib/utils/token-props.js | Replaced module.exports with export const definitions |
lib/rules/use-tokens/utils.js | Added .js extensions on imports from token-props |
lib/rules/use-tokens/parseShorthand.js | Updated import to ./utils.js |
lib/rules/use-tokens/index.js | Updated imports to include .js extensions |
lib/rules/use-colors/index.js | Updated import to token-utils.js |
index.js | Updated plugin import to index.js path |
.eslintrc.json | Disabled import/extensions rule to allow .js paths |
Comments suppressed due to low confidence (2)
package.json:14
- The package.json is missing a "type": "module" field, which is required for Node to treat .js files as ES modules. Add "type": "module" at the root of package.json to enable strict ESM.
"exports": {
.eslintrc.json:4
- [nitpick] Consider re-enabling and configuring the
import/extensions
rule to enforce consistent.js
extensions instead of disabling it entirely.
"import/extensions": "off"
8528baa
to
941d323
Compare
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.
LGTM.
Part of upgrading dependencies for web-foundations (Nx plugin needs Stylelint 16 which is currently blocked because our plugins don't use strict ESM)