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

chore(editools)!: upgrade @keystone-6/core to 5.2.0 #379

Merged
merged 6 commits into from
May 24, 2023

Conversation

nickhsine
Copy link
Contributor

@nickhsine nickhsine commented May 18, 2023

BREAKING CHANGE:
GQL schema is changed due to @keystone-6/core upgrade.
Mainly, fields, such as image_mode, file_mode, which are generated by image, file field type are effected.

Related PR:
#378

@nickhsine nickhsine changed the title refactor(editools): upgrade @keystone-6/core to 5.2.0 !refactor(editools): upgrade @keystone-6/core to 5.2.0 May 18, 2023
@nickhsine
Copy link
Contributor Author

@caesarWHLee @erase2004 有空請幫忙看一下 PR

Copy link
Contributor

@erase2004 erase2004 left a comment

Choose a reason for hiding this comment

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

確認到以下問題,可一併處理,或另外處理

  • field level 的 ACL 需要改寫:

  • dfaultValue 應為 defaultValue,或者移除該設定

    • 受影響 List: LiveblogItem, Question
  • sortOrder 的 dfaultValue 應為 defaultValue,而且 false 無法指派為 integer,所以看是用 0 作為預設值,或是刪除這個設定

    • 受影響 List: Field, FieldOption,
  • ui > displayMode 數值錯誤,不存在 display 選項,應為 cards, select 或 count

    • 受影響 List: FieldOption, Form, FormAnswer, FormResult, Liveblog, Publisher
  • Publisher list 中存在額外錯誤

    • 在同一個 ui 底下,重複的 key 的錯誤,e.g., linkToItem, inlineCreate
    • isindexed 應為 isIndexed, 而且數值為 boolean
    • displayoOde 應為 displayMode
    • resolveInput 中有 type error

    另外,file 和 image 影響到的 list,上到 dev 後,須確認上傳後的網址資訊是否正確。

@@ -57,7 +58,7 @@ const listConfigurations = list({
disabledButtons: [],
website: 'readr',
}),
heroImage: customFields.relationship({
heroImage: relationship({
label: '首圖',
ref: 'Photo',
customConfig: {
Copy link
Contributor

Choose a reason for hiding this comment

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

customConfig 應移除?

@@ -72,7 +73,7 @@ const listConfigurations = list({
},
},
}),
mobileImage: customFields.relationship({
mobileImage: relationship({
label: '手機首圖',
ref: 'Photo',
customConfig: {
Copy link
Contributor

Choose a reason for hiding this comment

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

customConfig 應移除?

Copy link
Contributor

Choose a reason for hiding this comment

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

這裡面有大量的錯誤:

  • ui > removeMode 的數值錯誤
  • 在同一個 ui 底下,重複的 key 的錯誤,e.g., linkToItem, inlineCreate
  • hook 應為 hooks,但 resolveInput 有使用到未被引入的 draftConverter

@@ -14,7 +14,7 @@ const listConfigurations = list({
label: '標題',
validation: { isRequired: true },
}),
imageFile: image(),
imageFile: image({ storage: 'images' }),
Copy link
Contributor

Choose a reason for hiding this comment

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


Snipaste_2023-05-19_13-42-56


Snipaste_2023-05-19_13-43-32

最大的影響應該是無法直接用藍色連結複製圖片連結,其他 (Copy Ref 或 Paste Ref) 可能還好,畢竟 Copy Ref 所複製出來的內容也有問題

Copy link
Contributor

Choose a reason for hiding this comment

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

在 Ln. 44 和 Ln. 121, 122 有 type error

Copy link
Contributor

Choose a reason for hiding this comment

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

這裡面有大量的錯誤:
在同一個 ui 底下,重複的 key 的錯誤,e.g., inlineCreate

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Ln. 99, Ln. 146 有 type error
  • Ln. 165 使用到不存在的 draftConverter ( customFields.draftConverter )

Copy link
Contributor

Choose a reason for hiding this comment

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

hook 應為 hooks,但 resolveInput 使用到不存在的 draftConverter ( customFields.draftConverter )

@nickhsine
Copy link
Contributor Author

上述的錯誤是在 @keystone-6/core@1.1.0 時就存在的問題,和升到 5.2.0 沒有直接的關係。
我的想法是我先把 pkg 升到 5.2.0 後,確定沒有問題,這些 warning 之後再找時間解。

@nickhsine nickhsine changed the title !refactor(editools): upgrade @keystone-6/core to 5.2.0 refactor(editools)!: upgrade @keystone-6/core to 5.2.0 May 19, 2023
@nickhsine
Copy link
Contributor Author

@caesarWHLee
我把 workspace 的設定改回原本的設定,涵括全部的 subpkgs。
本來將 packages/editools, packages/core 移出 workspace 的用意,是避免影響到其他人的開發體驗。
但後來測試發現, 不移出 workspace 也無妨,所以新的 commit 22cc1dc 把設定更改回去。

nickhsine added a commit to nickhsine/Lilith that referenced this pull request May 22, 2023
nickhsine added a commit to nickhsine/Lilith that referenced this pull request May 24, 2023
@nickhsine
Copy link
Contributor Author

nickhsine commented May 24, 2023

@caesarWHLee 我把 workspace 的設定改回原本的設定,涵括全部的 subpkgs。 本來將 packages/editools, packages/core 移出 workspace 的用意,是避免影響到其他人的開發體驗。 但後來測試發現, 不移出 workspace 也無妨,所以新的 commit 22cc1dc 把設定更改回去。

後來再測一次發現,
如果不把 packages/editools 移出 workspaces 的話,會遇到問題。
以下是 yarn install 時會遇到的錯誤 logs:

Ensure that there is only one instance of "graphql" in the node_modules
directory. If different versions of "graphql" are the dependencies of other
relied on modules, use "resolutions" to ensure only one version is installed.

https://yarnpkg.com/en/docs/selective-version-resolutions

Duplicate "graphql" modules cannot be used at the same time since different
versions may have different capabilities and behavior. The data from one
version used in the function from another could produce confusing and
spurious results.
    at instanceOf (/Users/lifa-hsien/Codes/mirror-media/Lilith/packages/editools/node_modules/@keystone-6/core/node_modules/graphql/jsutils/i
nstanceOf.js:44:19)
    at isScalarType (/Users/lifa-hsien/Codes/mirror-media/Lilith/packages/editools/node_modules/@keystone-6/core/node_modules/graphql/type/de
finition.js:117:37)
    at Object.isLeafType (/Users/lifa-hsien/Codes/mirror-media/Lilith/packages/editools/node_modules/@keystone-6/core/node_modules/graphql/ty
pe/definition.js:280:10)
    at /Users/lifa-hsien/Codes/mirror-media/Lilith/packages/editools/node_modules/@keystone-6/core/fields/dist/keystone-6-core-fields.cjs.dev
.js:3776:19
    at getListsWithInitialisedFields (/Users/lifa-hsien/Codes/mirror-media/Lilith/packages/editools/node_modules/@keystone-6/core/dist/types-
for-lists-77f3ad6c.cjs.dev.js:1418:17)
    at Object.initialiseLists (/Users/lifa-hsien/Codes/mirror-media/Lilith/packages/editools/node_modules/@keystone-6/core/dist/types-for-lis
ts-77f3ad6c.cjs.dev.js:1865:25)
    at Object.createSystem (/Users/lifa-hsien/Codes/mirror-media/Lilith/packages/editools/node_modules/@keystone-6/core/dist/createSystem-7a9
bb5d5.cjs.dev.js:2176:31)

@caesarWHLee 研究了一陣子,
發現是 yarn workspace 在處理 subpkgs 有不同版本的 dependencies 上有問題。

因為 pacakges/editools 需要的

  • @keystone-6/core 版本是 @5.2.0
  • @keystone-6/auth 版本是 @7.0.0
    pacakges/editools 依賴的 @mirrormedia/lilith-core@2.0.0-alpha.4,會被 yarn workspace soft link 到 packages/core

pacakges/readr 等其他 subpkg 則依賴

  • @keystone-6/core@1.1.1
  • @keystone-6/auth@1.1.0
  • @mirrormedia/lilith-core@1.2.8 (lilith-core@1.2.8 也依賴 @keystone-6/core@1.1.1

因為 @keystone-6/core@1.1.1@keystone-6/auth@1.1.0 需要的 graphql 是 15.8.0,
@keystone-6/core@5.2.0@keystone-6/auth@7.0.0 需要的 graphql 是 16.6.0,
yarn workspace 在處理以上不同版的 dependencies 時,會將 grahpql (core, auth 的 dependency) 安裝到多個地方。
因為一個 application 使用 graphql dep 時,需要使用同一個版本的 graphql,所以就報了以上的錯誤。

目前和文瀚推測是 yarn 1.* 的 bug,所以這邊就不再花時間研究怎麼處理,
先暫時將 packages/editools 移出 workspace。

nickhsine added a commit to nickhsine/Lilith that referenced this pull request May 24, 2023
BREAKING CHANGE:

This patch upgrades @keystone-6/core dependency, which
changes the `image` field type on the web UI.

Upgrade KeystoneJS 6 dependencies
- @keystone-6/auth: 1.0.0 -> 7.0.0
- @keystone-6/core: 1.1.1 -> 5.2.0

Since @mirrormedia/lilith-core also depends on @keystone-6/core,
therefore, we also need to upgrade lilith-core. Those changes in
lilith-core will be in another PR. Here, we temparorily upgrade
lilith-core to the workable alpha version.

Fix dependencies version to avoid multiple React copies problems
- next@13.3.4
- react@18.2.0
- react-dom@18.2.0
- replace `custom.relationship` by built-in `relationship` field
type.
- replace `custom.timestamp` by built-in `timestamp` field type
- update `ui.views`
- update `image` field arguments
- udpate `file` field argumenets
@nickhsine nickhsine merged commit 8308cf7 into mirror-media:main May 24, 2023
@nickhsine nickhsine changed the title refactor(editools)!: upgrade @keystone-6/core to 5.2.0 chore(editools)!: upgrade @keystone-6/core to 5.2.0 Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants