Closed ephys closed 2 years ago
Hi and thanks. I took a look and there's a test issue as well as a couple of things I'd like to understand:
rxjs
(now an explicit peerDependency
) is not part of package.json
. This needs to be fixed.eslint
- what is the scenario in which @rushstack/eslint-patch
is required?assert()
checks - is this due to the new typescript ~4.4.4
requirement introducing additional checks?assert()
checks - it looks like this brings about the need to turn on esModuleInterop
. I've had all sorts of weird things happen after turning this on, albeit for larger projects. All in all I like to leave TS compiler flags alone unless there's a pressing need.Can you explain the need for the other changes? I'll happily accept a PR containing the Nest 8 dependency support alone for now.
Hi
eslint
does not technically support loading plugins that are not direct children of node_modules
, and npm
does not guarantee a fully flattened node_modules
: a plugin installed by a preset might be in /node_modules/the_preset/node_modules/the_plugin
instead of /node_modules/the_plugin
.
eslint-patch
makes eslint search for plugins in the node_modules
of presets too.
Assert check is indeed due to a typescript change: https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#use-unknown-catch-variables.
I can remove esModuleInterop
, but I then need to replace import assert from 'assert';
with either:
import * as assert from 'assert';
(note: will produce invalid code if you ever target ES Modules instead of CJS as assert is a module, not a function)import assert = require('assert');
(compilation will fail if you ever target ES Modules instead of CJS)Which one do you prefer?
note: same with import * as Joi from 'joi';
There is no need for the other changes, they're there as a "while I'm updating that dep, might as well update all of them". If you don't want them feel free to close the PR. Supporting nest 8 is as simple as setting the peerDeps to
"@nestjs/common": "^8.0.0 | ^7.0.0", "@nestjs/core": "^8.0.0 | ^7.0.0",
As it's already compatible with nest 8
Hi, thanks for your reply. I've pushed a new version for NestJS ^8.0.0
compatibility - thanks for testing it our beforehand.
I'm still interested in the other changes.
import * as assert from 'assert';
for now, we'll deal with ESM problems when they ariseeslint
patch/fix is fine toorxjs
I'll happily accept a PR concerning these points (minus the now obsolete NestJS 8 peerDependency
)Have not heard back, closing as stale.
This PR updates the dependencies used by this project and marks nest 8 as a supported peer dependency.
No actual changes to the source code were required so nest 7 is still marked as supported.
Other notes:
@rushstack/eslint-patch
to fix an issue with eslint not finding the plugins imported by@elunic/eslint-config-ecs
.pre-commit
config has moved due to upgrading husky