WIP feat(perf): Upgrade typeorm version from v0.3.11 to v0.3.20 #2686

monrostar · 2024-02-20T13:47:00Z

Description

Please include a summary of the changes and the related issue.

Breaking changes

Does this PR include any breaking changes we should be aware of?

Screenshots

You can add screenshots here if applicable.

Checklist

📌 Always:

I have set a clear title
My PR is small and contains a single feature
I have checked my own PR

👍 Most of the time:

I have added or updated test cases
I have updated the README if needed

…ColumnName

monrostar · 2024-02-20T13:49:50Z

The first change that needed to be made was to replace connection.namingStrategy.eagerJoinRelationAlias
because it was removed in this commit fix: resolve issue building eager relation alias for nested relations

the current state of vendure/core tests is such after the changes. Now I will edit the data types for QueryBuilder. All errors indicate that there were changes in typeorm that broke the backward capability with the current version

cross-env PACKAGE=core vitest --config ../../e2e-common/vitest.config.ts --run

…hods

monrostar · 2024-02-20T17:22:38Z

New update regarding core tests

cross-env PACKAGE=core vitest --config ../../e2e-common/vitest.config.ts --run

asonnleitner · 2024-02-20T17:30:43Z

@michaelbromley Could this PR also include changing the name of connection to dataSource and all related items, such as renaming dbConnectionOptions to dataSourceOptions, etc.? As mentioned here:

https://github.com/typeorm/typeorm/blob/83567f533482d0170c35e2f99e627962b8f99a08/src/data-source/DataSource.ts#L47-L54

Additionally, considering the naming convention, TransactionalConnection could be renamed to TransactionalDataSource, among other related changes. Both the connection and dataSource terminologies can be used for some time, with the connection-related items being annotated with @deprecated to indicate their phased removal.

…tion}` -> `__{relation}`

monrostar · 2024-02-20T18:42:29Z

Unfortunately, it turned out to be impossible to use DefaultNameStrategy from TypeOrm because inside SQL eager relations we have Alias' name with two __ and in DefaultNameStrategy with only one _.

So we have a difference of one underscore _.

For this variant of events, we can create our NamingStrategy pattern. But maybe there exists the way to remove double underscores

example:

SELECT
"facet". "createdAt" AS "facet_createdAt",
"facet" "updatedAt" AS "facet_updatedAt", "facet" "isPrivate" AS "facet_isPrivate", "facet". "code" AS "facet_code", "facet". "id" AS "facet_id", "face
__translations". "createdAt" AS "facet__translations_createdAt" ,
"facet__translations" "updatedAt" AS "facet__translations_updatedAt", "facet__translations" "languageCode" AS "facet__translations_la
nguageCode",
"facet__translations". "name" AS "facet__translations_name", "facet__translations". "id" AS "facet__translations_id",
"facet__translations". "baseId" AS "facet__translations_baseId" FROM "
public". "facet" "facet" LEFT JOIN "public". "facet_translation" "facet__translations" ON "facet__translations". "baseId"="facet". "id" LEFT JOIN "public". "facet_channels_channel" "facet_lqb__channel"
ON "facet_lqb__channel". "facetId"="facet". "id" LEFT JOIN "public". "channel" "Zqb__channel" ON "Zqb__channel". "id"="facet_lqb__channel". "channelId" WHERE (facet_translations name ILIKE $1) AND "Zqb__
channel". "id" = $2 ORDER BY "facet" "createdAt" DESC

Progress in tests:

cross-env PACKAGE=core vitest --config ../../e2e-common/vitest.config.ts --run

michaelbromley · 2024-02-21T07:41:39Z

@monrostar great progress! Thanks for keeping us updated.

@asonnleitner regarding renaming to "dataSource" - I don't see this as a priority at the moment. We could support both, e.g. have config.dataSourceOptions be an alias to config.dbConnectionOptions but I don't see a compelling reason to add the complexity. It means more code to handle 2 code paths internally, and a bigger API surface for users.

monrostar · 2024-02-21T10:47:10Z

Progress in tests:

cross-env PACKAGE=core vitest --config ../../e2e-common/vitest.config.ts --run

At the moment, most of the tests that failed are related to a filtration error that I will soon solve

monrostar · 2024-03-01T11:03:40Z

42764b4
Related issue typeorm/typeorm#10063
This commit includes entity relationship changes because they are mostly related to ManyToOne, ManyToMany or other relationships that don't have a reverse side relationship for relationLoadStrategy: 'query'. Because the reverse relations were missing, Typeform cannot optimize queries on its own.

Progress in tests:

cross-env PACKAGE=core vitest --config ../../e2e-common/vitest.config.ts --run

…ug in Typeorm

monrostar · 2024-03-01T16:25:45Z

ddaaaf2
A related issue in the Collection entity Tree decorators: typeorm/typeorm#9936

Special fixes and refactoring for the test case in e2e for nesting in collections

vendure/packages/core/e2e/collection.e2e-spec.ts

Line 637 in 39fe610

it('nested parent field in shop API', async () => {

@michaelbromley Please review this code. Do you agree with my solution?

The tests that remain to be fixed

…rk around bugs in collections

monrostar · 2024-03-02T14:48:49Z

ddaaaf2 A related issue in the Collection entity Tree decorators: typeorm/typeorm#9936

Special fixes and refactoring for the test case in e2e for nesting in collections

vendure/packages/core/e2e/collection.e2e-spec.ts

Line 637 in 39fe610

it('nested parent field in shop API', async () => {

@michaelbromley Please review this code. Do you agree with my solution?
The tests that remain to be fixed

I would suggest adding property to the system to add a link to relation properties that have Tree decorators, then we can work around this bug until it is fixed. Like I was added collection and collection.children and collection.parent, but we can do it more dynamically
This can be a temporary setting in vendureConfig with which you can identify entities that have these problems and just create joins according to the scheme I created

monrostar · 2024-03-03T12:15:40Z

ddaaaf2 A related issue in the Collection entity Tree decorators: typeorm/typeorm#9936
Special fixes and refactoring for the test case in e2e for nesting in collections

vendure/packages/core/e2e/collection.e2e-spec.ts

Line 637 in 39fe610

it('nested parent field in shop API', async () => {

@michaelbromley Please review this code. Do you agree with my solution? The tests that remain to be fixed

I would suggest adding property to the system to add a link to relation properties that have Tree decorators, then we can work around this bug until it is fixed. Like I was added collection and collection.children and collection.parent, but we can do it more dynamically This can be a temporary setting in vendureConfig with which you can identify entities that have these problems and just create joins according to the scheme I created

32e0df7
@michaelbromley
This is what I was able to achieve when I made this function fully dynamic, now even if someone adds new Tree entities it won't be a problem.

monrostar · 2024-03-03T14:04:33Z

f9f5260
Fixes for filtering on translations that were used in join with different aliases due to incorrect aliases

In the method, I started using the original qb.alias everywhere, and also replaced rawConnection with qb.connection

monrostar · 2024-03-04T11:30:35Z

2219b44

In this test, we have a preset of only 2 variants. I don't understand why the test was for 6 calls if we have only 2 calls to the strategy

@michaelbromley It was your commit, so I need to make sure I didn't break any logic here that you were adding.

monrostar@a09c2b2

michaelbromley · 2024-03-04T11:56:31Z

@monrostar This is interesting. At first I thought the 6 calls was due to the fact that each variant is returning 3 price fields. But thinking about it further, the price and priceWithTax fields on the variant are derived from the one true price value in prices.price.

So yes, 2 calls to the transformer does indeed make more sense.

Yet, in the current version (pre your changes to TypeORM), it is indeed called 6 times. I logged the calls inside the transformer:

from: (databaseValue: string): number => {
  console.log(`transformerFromSpy: ${databaseValue}`);
  // ...
}

and got:

transformerFromSpy: 31
transformerFromSpy: 999999900
transformerFromSpy: 31
transformerFromSpy: 999999900
transformerFromSpy: 31
transformerFromSpy: 999999900

so it looks like the transformer gets called 3x for each variant. Totally strange. No idea why this would change now, but the change seems to be correct.

…tions

monrostar · 2024-03-06T15:06:54Z

b6b6663

I have now finalized this PR, I need you to check this code and also try to fix or find the problem in this test. I couldn't find a solution for this test, maybe it is the configuration that is the problem

vendure/packages/core/e2e/list-query-builder.e2e-spec.ts

Line 1190 in 8f9fd98

describe('customPropertyMap', () => {

@michaelbromley

monrostar · 2024-03-06T16:16:18Z

I also want to try this configuration at the driver level to find the remaining problematic queries related to collections

michaelbromley · 2024-03-12T08:47:26Z

Closed by 0afc94e

Excellent work, thank you very much!

monrostar added 2 commits February 20, 2024 14:36

feat(core): Upgrade typeorm version from v0.3.11 to v0.3.20

e9b1169

feat(core): Change removed method eagerJoinRelationAlias to joinTable…

6ebbcfc

…ColumnName

monrostar changed the title ~~feat(core): Upgrade typeorm version from v0.3.11 to v0.3.20~~ WIP feat(core): Upgrade typeorm version from v0.3.11 to v0.3.20 Feb 20, 2024

feat(core): Added relations from queries and removed optimization met…

dd30d22

…hods

feat(core): Fixed alias from NamingStrategy due to difference `_{rela…

937d4de

…tion}` -> `__{relation}`

monrostar added 2 commits February 21, 2024 10:43

feat(core): Update translationsAlias and clean up code from comments

ae6d95d

feat(core): Update expression for product-variant.entity.ts

4a9e082

feat(core): Update inverse relations and filter for entities

42764b4

feat(core): Refactoring for a problem with Tree collection due to a b…

ddaaaf2

…ug in Typeorm

monrostar changed the title ~~WIP feat(core): Upgrade typeorm version from v0.3.11 to v0.3.20~~ WIP feat(perf): Upgrade typeorm version from v0.3.11 to v0.3.20 Mar 1, 2024

monrostar added 2 commits March 1, 2024 18:10

feat(core): Backward compatibility and ability to use old logic to wo…

e9762ed

…rk around bugs in collections

feat(chore): Fix comment for relationLoadStrategy

2c37221

feat(core): Refactoring and extending the tree join function

32e0df7

feat(core): Fix for un-joined relations or joined with wrong alias

f9f5260

monrostar added 5 commits March 3, 2024 15:42

chore(core): Clean up code

0d8fec0

chore(core): Clean up code

47add43

feat(core): Add missing reverse relations

ce7be86

chore(core): Clean up code

752e31a

feat(core): Fix test case because we have only 2 variants of product

2219b44

monrostar added 2 commits March 6, 2024 12:39

feat(core): Refactoring for list-query-builder.ts and add missed rela…

3801422

…tions

feat(core): Add unjoined translation relations and fix alias for filters

b6b6663

monrostar marked this pull request as ready for review March 6, 2024 15:07

feat(core): Improve joinTreeRelationsDynamically function

0c82208

michaelbromley closed this Mar 12, 2024

monrostar deleted the feat/upgrade-typeorm branch March 16, 2024 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP feat(perf): Upgrade typeorm version from v0.3.11 to v0.3.20 #2686

WIP feat(perf): Upgrade typeorm version from v0.3.11 to v0.3.20 #2686

monrostar commented Feb 20, 2024 •

edited

monrostar commented Feb 20, 2024 •

edited

monrostar commented Feb 20, 2024 •

edited

asonnleitner commented Feb 20, 2024

monrostar commented Feb 20, 2024 •

edited

michaelbromley commented Feb 21, 2024

monrostar commented Feb 21, 2024 •

edited

monrostar commented Mar 1, 2024 •

edited

monrostar commented Mar 1, 2024 •

edited

monrostar commented Mar 2, 2024 •

edited

monrostar commented Mar 3, 2024 •

edited

monrostar commented Mar 3, 2024 •

edited

monrostar commented Mar 4, 2024 •

edited

michaelbromley commented Mar 4, 2024

monrostar commented Mar 6, 2024

monrostar commented Mar 6, 2024

michaelbromley commented Mar 12, 2024

WIP feat(perf): Upgrade typeorm version from v0.3.11 to v0.3.20 #2686

WIP feat(perf): Upgrade typeorm version from v0.3.11 to v0.3.20 #2686

Conversation

monrostar commented Feb 20, 2024 • edited

Description

Breaking changes

Screenshots

Checklist

monrostar commented Feb 20, 2024 • edited

monrostar commented Feb 20, 2024 • edited

asonnleitner commented Feb 20, 2024

monrostar commented Feb 20, 2024 • edited

michaelbromley commented Feb 21, 2024

monrostar commented Feb 21, 2024 • edited

monrostar commented Mar 1, 2024 • edited

monrostar commented Mar 1, 2024 • edited

monrostar commented Mar 2, 2024 • edited

monrostar commented Mar 3, 2024 • edited

monrostar commented Mar 3, 2024 • edited

monrostar commented Mar 4, 2024 • edited

michaelbromley commented Mar 4, 2024

monrostar commented Mar 6, 2024

monrostar commented Mar 6, 2024

michaelbromley commented Mar 12, 2024

monrostar commented Feb 20, 2024 •

edited

monrostar commented Feb 20, 2024 •

edited

monrostar commented Feb 20, 2024 •

edited

monrostar commented Feb 20, 2024 •

edited

monrostar commented Feb 21, 2024 •

edited

monrostar commented Mar 1, 2024 •

edited

monrostar commented Mar 1, 2024 •

edited

monrostar commented Mar 2, 2024 •

edited

monrostar commented Mar 3, 2024 •

edited

monrostar commented Mar 3, 2024 •

edited

monrostar commented Mar 4, 2024 •

edited