-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
!!!TASK: Remove plow-js #3306
!!!TASK: Remove plow-js #3306
Conversation
7020c93
to
8603b0c
Compare
10dd28a
to
4d1baf9
Compare
6cc67f9
to
9a47f88
Compare
8ec508f
to
d64b583
Compare
we could also merge this into 8.3 already and remove the extensibility export of plow starting with 9.0 ... and due to plows frequent usage, the neos extensibility api could augment all plow methods with a deprecated warning, so plugin authors and users know what to expect with neos 9 :) |
@@ -249,6 +249,8 @@ public function getDisallowedProperties(NodeInterface $node): array | |||
}; | |||
|
|||
$disallowedProperties = array_filter(array_keys($node->getNodeType()->getProperties()), $filter); | |||
$disallowedProperties = array_values($disallowedProperties); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi :D why has this been added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array_filter
leaves keys intact. Through json_encode
this can lead to arrays being serialized as objects. array_values
makes sure that we always end up with a strictly numerical array.
id like to merge this before we tackle: #3217 as there will be (some easy to fix) merge conflicts. Im planning the css rename for 8.3 ... also the earlier we merge, the more we can test ^^ |
#3304 will result in a small merge conflict ;) |
it was discussed that PluginViewEditor will be deprecated with 8.3 |
will resolve merge conflicts |
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; | |||
import {produce} from 'immer'; | |||
import mapValues from 'lodash.mapvalues'; | |||
import {connect} from 'react-redux'; | |||
import {$get, $contains, $set} from 'plow-js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
biggest merge conflict was in this file (packages/neos-ui/src/Containers/RightSideBar/Inspector/index.js)
because of @JamesAlias and @markusguenther work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you @grebaldi also did some refactoring here (extract preprocessViewConfiguration into evaluateExpression), i suspect we need to migrate this file again and start fresh
const addPlugin = (Plugin, isEnabled) => (ckEditorConfiguration, options) => { | ||
// we duplicate editorOptions here so it would be possible to write smth like `$get('formatting.sup')` | ||
if (!isEnabled || isEnabled(options.editorOptions, options)) { | ||
ckEditorConfiguration.plugins = ckEditorConfiguration.plugins || []; | ||
return $add('plugins', Plugin, ckEditorConfiguration); | ||
return { | ||
...ckEditorConfiguration, | ||
plugins: [ | ||
...(ckEditorConfiguration.plugins ?? []), | ||
Plugin | ||
] | ||
}; | ||
} | ||
return ckEditorConfiguration; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highlighting this addPlugin
utility function for everyone migrating $add
to normal js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a full "by reading" review of this once and now seeing that the e2e tests also succeed i give this green light ;)
# Conflicts: # Classes/Service/NodePolicyService.php # packages/neos-ui-sagas/src/Changes/index.js # packages/neos-ui-sagas/src/Publish/index.js # packages/neos-ui/src/Containers/PrimaryToolbar/PublishDropDown/index.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the PR and resolved conflicts. Also tested the PR locally. Thanks to @grebaldi for all the work.
87fa293
to
f34f62c
Compare
solves: #3118
I'm moving from package to package. Until the last package is finished, this remains a WIP.
TODOs
@neos-project/neos-ui-sagas
@neos-project/neos-ui
plow-js
as a runtime dependency for plugins. Removing plow from here will break every plugin that usesplow-js
itself.What do we do about that?I think it's feasible to offer plugin vendors support with this. I con open PRs for the most common plugins that use plow in this manner. Everyone else needs to be warned in advance, while 9.0 needs to be released with proper instructions. If removing plow is not feasible, Plugin vendors basically only need to addplow-js
as an explicit dependency and rebuild.Yoast.YoastSeoForNeos
: TASK: Remove dependency onplow-js
Yoast/Yoast-SEO-for-Neos#92Psmb.FlatNav
: https://github.com/psmb/Psmb.FlatNavPsmb.SplitAdd
: https://github.com/psmb/Psmb.SplitAddBreadlesscode.SimpleEditorExtend
: https://github.com/breadlesscode/neos-simple-editor-extendVisol.Neos.LinkClass
: https://github.com/visol/Visol.Neos.LinkClassUser impersonation needs testingClientEval:
in Inspector needs testing:neos-ui/packages/neos-ui/src/Containers/RightSideBar/Inspector/index.js
Lines 155 to 160 in 1f4766c
@neos-project/neos-ui-backend-connector
@neos-project/neos-ui-ckeditor5-bindings
tableCreationGrid
absolutely #3321)@neos-project/neos-ui-contentrepository
@neos-project/neos-ui-editors
RichTextEditor
causes UI crash whenformattingOptions
are not configured (unrelated to this PR) -> this needs more testing once it works properlyMasterPluginEditor
PluginViewEditor
PluginViewsEditor
CreateNew
decoratorDon't know how totestLinkInputOptions
-> possibly dead code?@neos-project/neos-ui-guest-frame
@neos-project/neos-ui-inspector
@neos-project/neos-ui-redux-store
neos-ui/packages/neos-ui-redux-store/src/CR/Nodes/selectors.ts
Lines 241 to 246 in 1f4766c
$get
reference while retaining the functionality ofgetPathInNode
through a manual replacement implementationgetPathInNode
from the Neos UI code basegetPathInNode
is up for removal in v10@neos-project/neos-ui-validators
@neos-project/neos-ui-views
$get
for data retrieval (as mentioned here: Remove PlowJS & immutable-js #3118 (comment))@neos-project/react-ui-components