decap-cms icon indicating copy to clipboard operation
decap-cms copied to clipboard

Get rid of Immutable.js

Open smashercosmo opened this issue 4 years ago • 9 comments

Is your feature request related to a problem? Please describe.

  1. Immutable.js is not maintained
  2. Immutable.js is quite big
  3. Immutable.js makes using netlify-cms less easy, especially when creating page previews, when you have to do things like
function BlogPostPagePreview({
  entry,
  widgetFor,
  fieldsMetaData,
}: {
  entry: any
  widgetFor: any
  fieldsMetaData: any
}) {
  const title = entry.getIn(['data', 'title']) as string
  const author = entry.getIn(['data', 'author']) as string
  const tags = entry.getIn(['data', 'tags']).map((tag: string) => {
    const slug = `/${slugify(tag.toLowerCase())}`
    return { tag, slug }
  })
  const cover = entry.getIn(['data', 'cover']) as string
  const body = widgetFor('body') as React.ReactElement
  const avatar = fieldsMetaData.getIn([
    'author',
    'authors',
    author,
    'avatar',
  ]) as string

  return (
    <BlogPostPage
      title={title}
      body={body}
      author={author}
      avatar={avatar}
      cover={cover}
      tags={tags}
    />
  )
}

Describe the solution you'd like Standard solution to deal with immutable data structures nowadays is Immer.js https://github.com/immerjs/immer

Additional context I know it will be a lot of work, because Immuatble.js is used across the whole codebase. And I really don't know how to introduce it gradually. But I'd really like to help.

smashercosmo avatar Jun 03 '20 13:06 smashercosmo

Huge thumbs up! This is a tech debt, but will require a breaking change so probably for v3

erezrokah avatar Jun 03 '20 13:06 erezrokah

Describe the solution you'd like Standard solution to deal with immutable data structures nowadays is Immer.js https://github.com/immerjs/immer

Additional context I know it will be a lot of work, because Immuatble.js is used across the whole codebase. And I really don't know how to introduce it gradually. But I'd really like to help.

We already have immer as a dependency so we could start by removing non API related usages of immutable. For example - we could change the reducers, but keep immutable when passing props to widgets.

erezrokah avatar Jun 03 '20 14:06 erezrokah

Hey, I want to start working on this. Could you guide me a bit? What would be the steps if I, for example, want to remove Immutable from https://github.com/netlify/netlify-cms/blob/master/packages/netlify-cms-core/src/reducers/auth.js?

smashercosmo avatar Jul 09 '20 16:07 smashercosmo

Thanks @smashercosmo! I would first search where the auth state is being accessed (probably using grep command on the core package).

For example here is one place: https://github.com/netlify/netlify-cms/blob/07f47824e960332d9be20588829d3895b32f000b/packages/netlify-cms-core/src/components/App/App.js#L264

Then replace those direct access to the auth immutable instance with selectors. For example instead of using auth && auth.get('user') have a function that receives the auth state and returns the user:

const selectUser = (auth) => auth && auth.get('user')

export { selectUser } 

We keep selectors in the same file as the relevant reducer, for example: https://github.com/netlify/netlify-cms/blob/07f47824e960332d9be20588829d3895b32f000b/packages/netlify-cms-core/src/reducers/entries.ts#L636

Once everything is behind selectors, you would mostly need to change the selectors internal implementation and it will be easier to get rid of Immutable.

Does that make sense?

erezrokah avatar Jul 09 '20 16:07 erezrokah

Yes. It does. Thank you)

smashercosmo avatar Jul 09 '20 16:07 smashercosmo

@erezrokah ok, I have a PR ready with immutable removed from 'status' and 'auth' state slices. What's the commit type for this kind of change?

smashercosmo avatar Dec 14 '20 14:12 smashercosmo

@smashercosmo that's great. I think you can use refactor: remove immutable.js from status and auth

erezrokah avatar Dec 14 '20 14:12 erezrokah

@martinjagodic is this still on the roadmap? Would you accept PRs which refactor immutable.js gradually out? (For all of the reasons stated in this issue, plus 3.5 years of tech debt interest)

mmkal avatar Jan 10 '24 20:01 mmkal

@mmkal that would be amazing, we would be very happy to accept this. However, this is a huge undertaking, so I guess bite-sized PRs would be a way to go. If you want, we can discuss this over Discord in more detail.

martinjagodic avatar Jan 11 '24 09:01 martinjagodic