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.

Sign up for GitHub

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

refactor: remove immutable from 'config' state slice #4960

Merged
merged 8 commits into from
Mar 11, 2021
Merged

refactor: remove immutable from 'config' state slice #4960

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7892db8
refactor: remove immutable from 'config' state slice
smashercosmo Feb 15, 2021
637819d
fix: add type to config's state
smashercosmo Feb 17, 2021
57b96f6
fix: remove non public types
erezrokah Feb 28, 2021
84366c6
refactor: don't use default state in tests
erezrokah Feb 28, 2021
c4fa7e8
refactor: remove todos
erezrokah Feb 28, 2021
652bebf
refactor: code cleanup
erezrokah Mar 10, 2021
3e3afde
chore: fix lint error
erezrokah Mar 10, 2021
c0e43bc
fix: maintain collection config order
erezrokah Mar 10, 2021
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
13 changes: 2 additions & 11 deletions packages/netlify-cms-core/src/reducers/entries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ function evaluateFolder(
entryMap: EntryMap | undefined,
field: EntryField | undefined,
) {
let currentFolder = config[folderKey];
let currentFolder = config[folderKey]!;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this is a correct way to handle it. Because here we're lying to compiler.

Copy link
Contributor

@erezrokah erezrokah Mar 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Under this function that value is always defined.
The only reason we allow not to set media_folder is if you have an external media library which in that case this code doesn't run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering if there is more correct way to express it with types, so we don't have to use these escape hatches.


// add identity template if doesn't exist
if (!collection.has(folderKey)) {
Expand All @@ -657,9 +657,6 @@ function evaluateFolder(
collection.get(folderKey)!,
entryMap,
collection,
// TODO can't figure out how to handle this
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
// @ts-ignore according to config type media_folder can be undefined
currentFolder,
folderKey,
config.slug,
Expand Down Expand Up @@ -705,9 +702,6 @@ function evaluateFolder(
collection.get(folderKey)!,
entryMap,
collection,
// TODO can't figure out how to handle this
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
// @ts-ignore according to config type media_folder can be undefined
currentFolder,
folderKey,
config.slug,
Expand Down Expand Up @@ -788,17 +782,14 @@ export function selectMediaFilePublicPath(
}

const name = 'public_folder';
let publicFolder = config[name];
let publicFolder = config[name]!;

const customFolder = hasCustomFolder(name, collection, entryMap?.get('slug'), field);

if (customFolder) {
publicFolder = evaluateFolder(name, config, collection!, entryMap, field);
}

// TODO can't figure out how to handle this
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
// @ts-ignore
return join(publicFolder, basename(mediaPath));
}

Expand Down