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

Remove PlowJS & immutable-js #3118

Closed
5 of 8 tasks
grebaldi opened this issue May 14, 2022 · 4 comments · Fixed by #3306
Closed
5 of 8 tasks

Remove PlowJS & immutable-js #3118

grebaldi opened this issue May 14, 2022 · 4 comments · Fixed by #3306

Comments

@grebaldi
Copy link
Contributor

grebaldi commented May 14, 2022

Motivation

Once upon a time, I've decided to create PlowJS, a library that would ease the handling of immutable state within the redux reducers of the Neos UI.

It seemed like a good idea at the time, but the JavaScript ecosystem has changed drastically since (and not exactly in favor of such a solution 😅).

There's several reasons why PlowJS has become a liability to the Neos UI code base:

  • PlowJS cannot be enhanced with TypeScript Typings without an amount of effort that would ultimately defeat the purpose
  • PlowJS treats JS-native datastructures and datastructures coming from immutable-js as the same thing (which is very bad in a type-safe world). The latter isn't needed anymore either, since it's runtime enforcement of immutability gives us very little benefit compared to static type checks.
  • Due to the points above, I've been neglecting maintenance of PlowJS for a couple of years already and I'm more inclined to archive it than to continue its development

The original purpose of PlowJS was to support the development of redux reducers. We already had a discussion about this a couple of years ago and decided to replace PlowJS with immer. Many (if not most) reducers have already been refactored since.

But PlowJS is being used everywhere because of it's convenient feature to hide the API of immutable-js from us.

The goal of this issue is therefore to remove both PlowJS and immutable-js entirely from the codebase.

Acceptance Criteria

  • No module uses PlowJS anymore
    • Whereever feasible, PlowJS has been replaced with immer
    • Whereever feasible, PlowJS has been replaced with proper Typings and optional chaining
  • No module uses immutable-js anymore
    • Whereever feasible, immutable-js has been replaced with native Object.freeze calls
    • Whereever feasible, immutable-js has been replaced with readonly-enforced Typings
  • No package manifest in the monorepo depends on PlowJS anymore
  • No package manifest in the monorepo depends on immutable-js anymore

References

@grebaldi
Copy link
Contributor Author

Seems like immutable js is already gone from the codebase. That'll make things easier :)

@mhsdesign
Copy link
Member

plow js is often used in the hoc @neos decorators, if we get around writing stuff in hooks, then we dont need plow-js in these places anymore ;)

@grebaldi
Copy link
Contributor Author

Bad News:

I've just noticed, that Plow is being leaked as API for Inspector Views. Here's an example:

https://github.com/neos/neos-ui/blob/8.3/packages/neos-ui-views/src/Data/ColumnView/index.js#L24

https://github.com/Flowpack/neos-matomo/blob/0cc2034d5b866b9860a0fad3521c4ea9299b7b9a/Configuration/NodeTypes.yaml#L90-L92

data is a dot-separated path being passed plainly to the view that will read the respective value from the result of the data source. This will stop working, once Plow JS is removed.

We will need to support this use case, but that doesn't require the entirety of Plow. It's probably best to have a utility function native to the UI code base that can replace $get for this purpose.

@grebaldi grebaldi self-assigned this Dec 22, 2022
@grebaldi grebaldi mentioned this issue Dec 22, 2022
36 tasks
@grebaldi grebaldi linked a pull request Dec 22, 2022 that will close this issue
36 tasks
@mhsdesign
Copy link
Member

Plugin vendors using esbuild or the alias feature directly can, if they want to bundle plow-js directly (to avoid deprecation warnings), exclude plow-js from the map:

const {"plow-js": _, ...extensibilityMap} = require("@neos-project/neos-ui-extensibility/extensibilityMap.json");

build({
   alias: extensibilityMap
})

@mhsdesign mhsdesign moved this to Under Review 👀 in Neos 9.0 Release Board Sep 1, 2023
@github-project-automation github-project-automation bot moved this from Under Review 👀 to Done ✅ in Neos 9.0 Release Board Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants