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

Move resourcepath logic onto serverside #2092

Open
mstruebing opened this issue Aug 23, 2018 · 5 comments
Open

Move resourcepath logic onto serverside #2092

mstruebing opened this issue Aug 23, 2018 · 5 comments
Labels

Comments

@mstruebing
Copy link
Contributor

As mentioned in this PR: #2087 (comment)

The logic to transform resource uris from yaml files should be moved onto the server side as the current one can lead to errors.

@grebaldi
Copy link
Contributor

grebaldi commented Jun 25, 2023

I've been looking into this and there's a couple of problems here:

  1. To resolve this on the server-side, we can either use a dedicated NodeTypePostprocessor or an Aspect. Both aren't good options performance-wise, because we would iterate through all configured node properties and resolve resource URIs (potentially costly). This operation would be performed every time the NodeType schema is resolved, even though the resolution of those resource URIs is only ever relevant to the UI and should not add cost to any userland code.
  2. The resolution of resource URIs (at least in the case of help thumbnails) is implemented incorrectly. A resource URI resource://Vendor.Site/Thumbnails/my-editor.png will be resolved to /_Resources/Static/Packages/Vendor.Site/Thumbnails/my-editor.png. This is wrong. What should resolve to /_Resources/Static/Packages/Vendor.Site/Thumbnails/my-editor.png is resource://Vendor.Site/Public/Thumbnails/my-editor.png (notice the /Public/ segment). This means that everybody who has configured help thumbnails via resource URI has done so with a wrong resource URI. We won't get out of that one without a breaking change for those people :(

We also need to consider other places in which resource URIs are manually resolved or /_Resources/Static/... paths are hard-coded. The places I found are:

  1. (no surprise) Property help thumbnails
  2. /_Resources/Static/Packages/Neos.Neos/Images/dummy-image.svg is hard coded in some places
  3. The <ResourceIcon/> component (introduced via FEATURE: Adds resource icon component  #2979)

For (1.) - considering the bigger picture - I'd suggest the following plan:

  1. Move Neos\Neos\Controller\Backend\SchemaController into Neos.Neos.Ui (while leaving the original intact - this is only to be able to apply UI-specifics to that endpoint)
  2. Resolve resource URIs in SchemaController

Case (2.) can be easily solved by either passing the dummy image URI as configuration or add the dummy-image.svg as a component to the build. I'd be fine with either one.

Case (3.) is more complicated, I'm afraid. The <ResourceIcon/> is a specialization of the (unfortunately public API) <Icon/>-component. We will need to deprecate its behavior, and (prior to that) find a way to allow for custom Icon URIs that can be resolved on the server side (we'll some sort of configurable mapping thing here).

What do you think of these suggestions?

/cc @mhsdesign @Sebobo @markusguenther @crydotsnake

@mhsdesign
Copy link
Member

mhsdesign commented Jan 19, 2024

Other new hard coded resource uris like for the node creation dialog also cause problems: #3616

Would it be tooo inefficient to have an endpoint for those edgecase / consumer defined icons that will A) either directly redirect them or B) return the correct path?

localhost:8081/neos/resolveResourceUri?path=resource://My.Package/Foo/bar.js

returns json with {"resolved":"http://localhost/_Resources/My.Package/Foo/bar.js"}
localhost:8081/neos/redirectToResourceUri?path=resource://My.Package/Foo/bar.js

redirects directly to http://localhost/_Resources/My.Package/Foo/bar.js

@grebaldi
Copy link
Contributor

I'd opt for B) with a 301 status - the browser will then leave the endpoint alone on all subsequent requests.

We must be careful with that though and append all additional query parameters to the redirect location to allow for cache-busting. 301 is cached quite aggressively by browsers iirc.

@mhsdesign
Copy link
Member

I am currently experimenting with B, and the first idea was a Schnapsidee as the request would be async and we wouldnt know what to do until the resource is loaded. Also two server roundtrips would be necessary.

Additionally while looking into the help thumbnail i found that there were two different type of thumbnails:

1

property help, this was a bit tricky as this option is currently not documented:

  properties:
    title:
      ui:
        help:
          message: "title"
          thumbnail: resource://Neos.Neos/Images/dummy-image.svg
image

2

on the node type level as documented here https://neos.readthedocs.io/en/stable/References/NodeTypeDefinition.html#nodetype-definition-reference

  ui:
    help:
      message: "node type"
      # note that we need the /Public/ part here!!! But the documentation above is also wrong :D
      thumbnail: resource://Neos.Neos/Public/Images/dummy-image.svg
image

The "global" help will be actually resolved on the server and additionally there is more magic in NodeTypeConfigurationEnrichmentAspect::resolveHelpMessageThumbnail:

If the thumbnail setting is undefined but an image matching the nodetype name is found, it will be used automatically. It will be looked for in /Resources/Public/NodeTypes/Thumbnails/.png with packageKey and nodeTypeName being extracted from the full nodetype name like this:

AcmeCom.Website:FooWithBar -> AcmeCom.Website and FooWithBar

This was introduced via:

neos/neos-development-collection@277989f

@mhsdesign
Copy link
Member

Update August 2024

So there have been now lots of discussions regarding this topic some directly with seb and Wilhelm some on GitHub and slack but its all spread out. So im going to conclude what our current approach to fix this issue in the longterm is:

We decided against any client based resource resolving possibly including server callbacks as laid out here #2092 (comment) and proposed via #3695
Aside from the additional complexity in the codebase the additional request showed some lags when using the inbuild php server for testing and this will probably also be the case for production on a slow connection. Also we would still have the dualism of having to resolve some icons still on the server because of the magic in resolveHelpMessageThumbnail so there might be no gain in having it partly on the client.

While we shortly evaluated using sprite svg to bundle all svgs efficiently it turned out to be rather complex in comparison to the gain - which would be bundle size and improved loading. As discussed in #3695 (comment) its simply not worth to use svg sprite for 4!!! icons and we will rather see to it how we can make font awesome more bundle friendly as it currently takes up 1.2mb (one third!!!) of the whole bundle.

Step 1

So our first step is to inline svgs in the final bundle which are hardcoded.
See #3836 which will implement this.

Step 2

As second step we will enhance the NodeTypeSchemaBuilder to transform all resource:// on the server.
That will get rid of all leftover usages leveraging client based resource resolving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants