Skip to content
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

fix(di): move locals arg to DIInvokeOptions #2890

Merged
merged 32 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
666a12a
chore: fix coverage configuration
Romakita Nov 17, 2024
f4fa686
refactor(di): move locals arg to DIInvokeOptions
Romakita Nov 15, 2024
1e94a01
feat(di): add Provider.getUseOpts
Romakita Nov 15, 2024
f7fa7d9
fix(di): remove unused resolvers options
Romakita Nov 15, 2024
18ce1e2
fix(di): remove injector.loadModule()
Romakita Nov 15, 2024
0a20009
fix(di): remove default scope configuration on DIConfiguration level
Romakita Nov 16, 2024
c157178
feat(hooks): create @tsed/hooks package
Romakita Nov 21, 2024
9e07d6e
refactor(di): remove new InjectorService in test
Romakita Nov 21, 2024
ec6971d
feat(di): add priority/alias props on Provider
Romakita Nov 22, 2024
f4af794
feat(di): add $beforeInvoke, $beforeInvoke:type, $afterInvoke
Romakita Nov 22, 2024
5eeabba
feat(di): injector.get resolve token when token isn't already cached
Romakita Nov 23, 2024
c13437a
fix(di): add global flag to register correctly provider on GlobalRegi…
Romakita Nov 24, 2024
3ae2919
fix(di): use DIConfiguration to cache configuration instance instead …
Romakita Nov 24, 2024
fb3288f
fix(di): make injector really a singleton
Romakita Nov 24, 2024
b9484d0
refactor(vike): improve dynamic typing import
Romakita Nov 21, 2024
ab1e239
refactor(platform-router): use functional API to inject service in ro…
Romakita Nov 21, 2024
aa6fd67
refactor(platform-params): use functional API to inject/declare services
Romakita Nov 22, 2024
e25e54f
refactor(platform-log-request): use functional API to inject/declare …
Romakita Nov 22, 2024
243bdce
refactor(platform-router): use hook to create router on the fly
Romakita Nov 22, 2024
f581b44
refactor(platform-exceptions): use functional API to inject/declare s…
Romakita Nov 23, 2024
5d9bfda
refactor(platform-response-filter): use functional API to inject/decl…
Romakita Nov 23, 2024
61f1640
refactor(platform-views): use functional API to inject/declare services
Romakita Nov 24, 2024
fc71af0
refactor(ajv): use functional API to inject/declare services
Romakita Nov 23, 2024
857ea09
refactor(swagger): use functional API to inject/declare services
Romakita Nov 23, 2024
51aba80
refactor(platform-http): use functional API to inject/declare services
Romakita Nov 22, 2024
ef4b43a
refactor(platform-express): use functional API to inject/declare serv…
Romakita Nov 23, 2024
4c9c32f
refactor(platform-koa): use functional API to inject/declare services
Romakita Nov 23, 2024
6acfaeb
refactor(normalize-path): move normalize-path module to third-parties
Romakita Nov 24, 2024
501ac71
refactor(platform-serverless): use functional API to inject/declare s…
Romakita Nov 23, 2024
d731171
fix(bullmq): change hook to handle correct DI initialization
Romakita Nov 24, 2024
edd143d
chore: update coverage
Romakita Nov 24, 2024
e2a2e85
ci: update test workflow
Romakita Nov 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 7 additions & 76 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,71 +33,6 @@ jobs:
- name: Run lint
run: yarn test:lint

test-integration:
runs-on: ubuntu-latest

strategy:
matrix:
node-version: [20.12.2]

steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: "yarn"
- name: Install dependencies
run: yarn install --immutable --network-timeout 500000
- name: Run build
run: yarn tsc --build
env:
FORCE_COLOR: true
- name: Run test
run: yarn test:integration
env:
FORCE_COLOR: true
- name: Upload vitest config files
uses: actions/upload-artifact@v4
with:
name: vitest-config-integration
overwrite: true
path: |
team.json
packages/platform/platform-express/vitest.config.mts
packages/platform/platform-koa/vitest.config.mts
continue-on-error: true

test-envs:
runs-on: ${{ matrix.os }}

strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
node-version: [20.12.2]

steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: "yarn"
- name: Install dependencies
run: yarn install --immutable --network-timeout 500000
- name: Run build
run: yarn tsc --build
env:
FORCE_COLOR: true
- name: Run test
run: yarn test:integration
env:
FORCE_COLOR: true

test-core:
runs-on: ubuntu-latest

Expand Down Expand Up @@ -190,8 +125,6 @@ jobs:
path: |
team.json
packages/platform/platform-*/vitest.config.mts
!packages/platform/platform-express/vitest.config.mts
!packages/platform/platform-koa/vitest.config.mts
test-orm:
runs-on: ubuntu-latest

Expand Down Expand Up @@ -311,8 +244,7 @@ jobs:

test-download-artifacts:
runs-on: ubuntu-latest
needs:
[lint, test-core, test-specs, test-platform, test-integration, test-envs, test-orm, test-security, test-graphql, test-third-parties]
needs: [lint, test-core, test-specs, test-platform, test-orm, test-security, test-graphql, test-third-parties]
if: github.event_name == 'pull_request'
strategy:
matrix:
Expand All @@ -337,8 +269,7 @@ jobs:

deploy-packages:
runs-on: ubuntu-latest
needs:
[lint, test-core, test-specs, test-platform, test-integration, test-envs, test-orm, test-security, test-graphql, test-third-parties]
needs: [lint, test-core, test-specs, test-platform, test-orm, test-security, test-graphql, test-third-parties]
if: github.event_name != 'pull_request' && contains('
refs/heads/production
refs/heads/alpha
Expand All @@ -360,11 +291,11 @@ jobs:
- name: Install dependencies
run: yarn install --network-timeout 500000

- name: Download Core vitest config files
uses: actions/download-artifact@v4
with:
pattern: vitest-config-*
merge-multiple: true
# - name: Download Core vitest config files
# uses: actions/download-artifact@v4
# with:
# pattern: vitest-config-*
# merge-multiple: true
- name: "Git status"
run: git status
- name: Release packages
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ lib-cov
# Coverage directory used by tools like istanbul
coverage
coverage-*


# Grunt intermediate storage (http://gruntjs.com/creating-plugins#storing-task-files)
.grunt

Expand Down
33 changes: 0 additions & 33 deletions docs/docs/configuration/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,45 +265,12 @@ Add providers or modules here. These modules or provider will be built before th
</Tab>
</Tabs>

### scopes

- type: `{[key: string]: ProviderScope}`

Change the default scope for a given provider. See [injection scopes](/docs/injection-scopes) for more details.

```typescript
import {Configuration, ProviderScope, ProviderType} from "@tsed/di";

@Configuration({
scopes: {
[ProviderType.CONTROLLER]: ProviderScope.REQUEST
}
})
export class Server {}
```

### logger

- type: @@PlatformLoggerSettings@@

Logger configuration. See [logger section for more detail](/docs/logger).

### resolvers - External DI

- type: @@DIResolver@@

Ts.ED has its own DI container, but sometimes you have to work with other DI like Inversify or TypeDI. The version
5.39.0+
now allows you to configure multiple external DI by using the `resolvers` options.

The resolvers options can be configured as following:

<<< @/docs/configuration/snippets/server-resolvers.ts

It's also possible to register resolvers with the @@Module@@ decorator:

<<< @/docs/configuration/snippets/module-resolvers.ts

### views

Object to configure Views engines with Ts.ED engines or Consolidate (deprecated). See more
Expand Down
13 changes: 0 additions & 13 deletions docs/docs/configuration/snippets/module-resolvers.ts

This file was deleted.

13 changes: 0 additions & 13 deletions docs/docs/configuration/snippets/server-resolvers.ts

This file was deleted.

6 changes: 2 additions & 4 deletions docs/docs/injection-scopes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

The scope of a [Provider](/docs/providers.md) defines the lifecycle and visibility of that bean in the context in which it is used.

Ts.ED DI defines 3 types of @@Scope@@ which can be used on injectable classes:
Ts.ED DI defines 3 types of @@ProviderScope@@ which can be used on injectable classes:

- `singleton`: The default scope. The provider is created during server initialization and is shared across all requests.
- `request`: A new instance of the provider is created for each incoming request.
Expand Down Expand Up @@ -75,9 +75,7 @@ Instance scope used on a provider tells the injector to create a new instance ea
With the functional API, you can also rebuild any service on the fly by calling @@inject@@ with the `rebuild` flag:

```typescript
import {inject, injector} from "@tsed/di";
import {inject} from "@tsed/di";

const myService = inject(MyService, {rebuild: true});
// similar to
const myService2 = injector().invoke(MyService, {rebuild: true});
```
Comment on lines +78 to 81
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider adding usage guidance for the rebuild flag.

While the code example is clear, it would be helpful to add a brief explanation of when developers should use the rebuild flag versus regular injection. This would help prevent misuse and potential performance issues.

Consider adding a note like this:

 import {inject} from "@tsed/di";
 
 const myService = inject(MyService, {rebuild: true});
+
+// Note: Use the rebuild flag when you need a fresh instance of a service,
+// typically for testing or when the service's state needs to be reset.
+// Be cautious with frequent rebuilds as they can impact performance.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import {inject} from "@tsed/di";
const myService = inject(MyService, {rebuild: true});
// similar to
const myService2 = injector().invoke(MyService, {rebuild: true});
```
import {inject} from "@tsed/di";
const myService = inject(MyService, {rebuild: true});
// Note: Use the rebuild flag when you need a fresh instance of a service,
// typically for testing or when the service's state needs to be reset.
// Be cautious with frequent rebuilds as they can impact performance.

Comment on lines +78 to 81
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

LGTM! Consider adding usage guidance for the rebuild flag.

The simplified import and example are clear and accurate. However, it would be helpful to add a brief explanation of when developers should consider using the rebuild flag, as this could have implications for service lifecycle and performance.

Consider adding context like:

 import {inject} from "@tsed/di";
 
 const myService = inject(MyService, {rebuild: true});
+
+// The rebuild flag is useful when you need to:
+// - Force creation of a new instance regardless of scope
+// - Reset the state of a service
+// Note: Use with caution as it bypasses the normal scoping rules
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import {inject} from "@tsed/di";
const myService = inject(MyService, {rebuild: true});
// similar to
const myService2 = injector().invoke(MyService, {rebuild: true});
```
import {inject} from "@tsed/di";
const myService = inject(MyService, {rebuild: true});
// The rebuild flag is useful when you need to:
// - Force creation of a new instance regardless of scope
// - Reset the state of a service
// Note: Use with caution as it bypasses the normal scoping rules

7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,13 @@
"test:ci": "yarn test:lint && yarn test:core && yarn test:specs && yarn test:platform && yarn test:integration && yarn test:graphql && yarn test:orm && yarn test:security && yarn test:third-parties",
"test:lint": "eslint '**/*.{ts,js}'",
"test:lint:fix": "eslint '**/*.{ts,js}' --fix",
"test:core": "lerna run test:ci --scope '@tsed/{core,di,platform-http,engines,normalize-path}' --stream --concurrency 2",
"test:platform": "lerna run test:ci --ignore '@tsed/platform-{express,koa}' --scope '@tsed/platform-*' --stream --concurrency 2",
"test:integration": "lerna run test:ci --scope '@tsed/platform-{express,koa}' --stream --concurrency 2",
"test:core": "lerna run test:ci --scope '@tsed/{core,di,hooks,engines}' --stream --concurrency 2",
"test:platform": "lerna run test:ci --scope '@tsed/platform-*' --stream --concurrency 2",
"test:orm": "lerna run test:ci --scope '@tsed/{adapters,adapters-redis,mikro-orm,mongoose,objection,prisma}' --stream --concurrency 4",
"test:graphql": "lerna run test:ci --scope '@tsed/{apollo,typegraphql}' --stream",
"test:security": "lerna run test:ci --scope '@tsed/{jwks,oidc-provider,passport,oidc-provider-plugin-wildcard-redirect-uri}' --stream",
"test:specs": "lerna run test --scope '@tsed/{ajv,exceptions,json-mapper,schema,swagger}' --stream --concurrency 2",
"test:third-parties": "lerna run test:ci --scope '@tsed/{agenda,bullmq,components-scan,event-emitter,formio,pulse,sse,socketio,stripe,temporal,terminus,vike,schema-formio,formio}' --stream --concurrency 1",
"test:third-parties": "lerna run test:ci --scope '@tsed/{normalize-path,agenda,bullmq,components-scan,event-emitter,formio,pulse,sse,socketio,stripe,temporal,terminus,vike,schema-formio,formio}' --stream --concurrency 1",
"coverage": "merge-istanbul --out coverage/coverage-final.json \"**/packages/**/coverage/coverage-final.json\" && nyc report --reporter text --reporter html --reporter lcov -t coverage --report-dir coverage",
"barrels": "lerna run barrels",
"build": "monorepo build --verbose",
Expand Down
59 changes: 0 additions & 59 deletions packages/core/src/domain/Hooks.spec.ts

This file was deleted.

Loading
Loading