-
Notifications
You must be signed in to change notification settings - Fork 3
release v3.0.0 of eslint-config (dependency upgrades) #28
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
|
Would it be okay if I make the changes related to the new release (a new package version) in this PR? Or do I need to create a separate PR? |
- Convert the eslint-config from CommonJS to ESM implementation - Make minor updates to the eslint.config.js rules to ensure that the linter pass
| 'ignorePackages', | ||
| { | ||
| js: 'never', | ||
| js: 'always', |
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.
Since, this is a behaviour change, I would suggest that we import the package from this PR locally into xrpl.js to make sure that linting passes or evaluate how much work needs to be done to fix linting errors that occurs after this change.
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.
I have ensured that this behavioral change only affects the files located inside this specific package. No downstream users of this package are affected by this change due to the regex used in the files section of eslint config file.
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.
updated in 75bab6b
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.
I see it's only suppressed for this repo.
No downstream users of this package are affected by this change due to the regex used in the files section of eslint config file.
The regex applies only for this repo. Did we try importing the package from this branch into xrpl.js to check if it works fine?
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.
No, I will do that in some time and get back to you. I wasn't sure how to import an unpublished package (this new version of typescript-style repo) into xrpl.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.
When importing a package locally, I would first delete node_modules and package-lock.json and install it fresh.
We can import it something like this:
"@xrplf/eslint-config": "file:<PACKAGE_PATH>"
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.
thanks
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.
Here is the PR that uses the new versions of dependencies: XRPLF/xrpl.js#3187
The linter passes for this PR (on my local computer, it might not clear the CI) and all the unit tests pass.
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.
Cool! I guess package-lock.json in this PR is outdated based on package version. Should regenerate it.
| "eslint-plugin-react": "^7.0.0", | ||
| "eslint-plugin-react-hooks": "^5.0.0", | ||
| "eslint-plugin-tsdoc": "^0.4.0", | ||
| "eslint-plugin-tsdoc": "^0.5.0", |
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.
^0.5.0 would make @xrplf/eslint-config release a breaking change. Consumers with eslint-plugin-tsdoc < 0.5.0 won't be able to install @xrplf/eslint-config. So when we release this, we should do major version bump of @xrplf/eslint-config
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.
…stream packages -- Relaxation of 'import/no-anonymous-default-export': 'off', which allows for more liberty in declaring default exports in all users of this package
…n this package only
packages/eslint-config/package.json
Outdated
| "eslint-config-prettier": "^10.0.0", | ||
| "eslint-import-resolver-typescript": "^4.0.0", | ||
| "eslint-plugin-array-func": "^4.0.0", | ||
| "eslint-plugin-array-func": "^5.1.0", |
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.
nit: We should keep peerDependencies ^5.0.0 for greater compatibility.
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.
ok f13ad20
Co-authored-by: Raj Patel <rajp@ripple.com>
…t-plugin-array-func; provide exports of necessary modules from the package.json used in xrpl.js monorepo
…escript-style into upgradeEslintPluginDep
This PR accomplishes the following:
eslint-plugin-tsdocdependency from 0.4.0 to 0.5.0.eslint-plugin-array-docfrom4.0.0to5.1.0. Due to this upgrade, theeslint-confighad to be converted from CommonJS to an ESM compatible implementation. This is because the said dependency only works with ESM modules.eslint-configrepository to indicate the upgrade of dependencies.Note: This PR is blocking the upgrade of xrpl.js mono repository with a similar dependency upgrade. The failing PR details can be found here: https://github.com/XRPLF/xrpl.js/pull/3176/changes