search

SAP / ui5-tooling

An open and modular toolchain to develop state of the art applications based on the UI5 framework
https://sap.github.io/ui5-tooling
Apache License 2.0
465 stars 69 forks source link

Default sandbox config missing in UI5 serve #256

Open deborsa opened 4 years ago

deborsa commented 4 years ago

Expected Behavior

After setting up SAPUI5 version 1.81.0 and running UI5 serve I should be able to serve the same files belonging to SAPUI5 as the content delivery server for all declared libraries so meaning http://localhost:8080/test-resources/... should bring the same results as https://sapui5.hana.ondemand.com/1.81.0/test-resources/...

Current Behavior

http://localhost:8080/test-resources/sap/ushell/shells/sandbox/fioriSandboxConfig.json brings a 404: Cannot GET /test-resources/sap/ushell/shells/sandbox/fioriSandboxConfig.json whereby https://sapui5.hana.ondemand.com/1.81.0/test-resources/sap/ushell/shells/sandbox/fioriSandboxConfig.json brings the correct file.

As a result I cannot run my App in sandbox mode because it relies on the default sandbox configuration which cannot be loaded. The sandbox itself under http://localhost:8080/test-resources/sap/ushell/bootstrap/sandbox.js and other SAPUI5 files are loaded fine so it seems my general setup is working.

Steps to reproduce the issue

  1. Prepare a ui5.yaml file: 
    specVersion: "2.2"
type: application
metadata:
name: [...]
framework:
name: SAPUI5
version: 1.81.0
libraries:
- name: sap.ui.core
- name: sap.m
- name: sap.ushell
  development: true
- name: themelib_sap_fiori_3
  2. UI5 serve
  3. Open URL http://localhost:8080/test-resources/sap/ushell/shells/sandbox/fioriSandboxConfig.json in browser

Context

Affected components (if known)

Log Output / Stack Trace

info normalizer:translators:ui5Framework Using SAPUI5 version: 1.81.0
Server started
URL: http://localhost:8080

The following stack trace when trying to open my app in sandbox mode in browser:

[...]
2020-08-26 16:37:15.817814 Mixing/Overwriting sandbox configuration from http://localhost:8080/test-resources/sap/ushell/bootstrap/../shells/sandbox/fioriSandboxConfig.json. -  
jquery.js?eval:9275 GET http://localhost:8080/test-resources/sap/ushell/shells/sandbox/fioriSandboxConfig.json 404 (Not Found)
send @ jquery.js?eval:9275
ajax @ jquery.js?eval:8756
sjax @ jquery.sap.sjax.js?eval:73
applyJsonApplicationConfig @ sandbox.js:141
bootstrap @ sandbox.js:215
constructor @ Core.js?eval:549
eval @ Core.js?eval:4231
(anonymous) @ ui5loader.js:1826
requireAll @ ui5loader.js:1749
executeModuleDefinition @ ui5loader.js:1800
ui5Define @ ui5loader.js:1932
eval @ Core.js?eval:8
execModule @ ui5loader.js:1664
requireModule @ ui5loader.js:1550
requireSync @ ui5loader.js:2059
(anonymous) @ flpSandbox.html:13
[...]
matz3 commented 4 years ago

The fioriSandboxConfig.json is not included by design. In general we don't include the test-resources/ folder into the npm packages which will be used when using the UI5 Tooling, because most of the content is not required for consumers and some libraries come with a huge set of test-resources.

For sap.ushell (and also sap.ui.core) this is a bit different. There are some files which are required for development / testing, so we've picked the relevant ones to also be included into the npm package. The fioriSandboxConfig.json references a lot of sample / demo applications which are all not packaged with the npm delivery. Therefore we didn't see a reason to include it.

Could you please give a bit context about your application scenario so that I can better understand your use case? We've check several applications and templates to ensure that all common scenarios are supported.

deborsa commented 4 years ago

My app triggers a cross application navigation to another app. The target app is configurable by the user in a productive environment. It is not part of my development. Therefore when testing the scenario in sandbox mode I add sap.ushell.demo.AppNavSample as the target for the intent in question. This app will just show all the passed parameter so I can verify that the navigation works fine. I can then also navigate back to my own app and verify that all scenarios work fine.

RandomByte commented 3 years ago

We discussed this in our team and think it's reasonable to include the fioriSandboxConfig.json as well as the AppNavSample app (~150 KB) in the existing sap.ushell package.

However, we have to check with the development team responsible for sap.ushell. Ultimately it should be their decision.

@deborsa besides sap.ushell.demo.AppNavSample, are you missing any other apps from the default sandbox configuration?

deborsa commented 3 years ago

For my current development the AppNavSample would be sufficient. I don't have requirements for any of the other sandbox apps.

RandomByte commented 3 years ago

Thanks. One more question: Do you actually require the fioriSandboxConfig.json as provided by sap.ushell?

I'm asking because if we include it without adding all the other demo apps referenced by it, developers might be even more confused. I guess with the AppNavSample resources you could come up with your own, project specific, sandbox config and work with that?

deborsa commented 3 years ago

I do not necessarily need the fioriSandboxConfig.json. I could also create my own sandbox config and include this navigation app. In my opinion it is not very nice though to always get a 404 with the default configuration because unfortunately the sandbox code has this config file hard coded.

Also I still think it would be better to be aligned with what the CDN delivers. In my opinion this should still be the same as what is available for the UI5 serve. I understand those other example apps are not really needed anyhow but then why even bother to serve them from the CDN? Maybe they could get removed alltogether also from the CDN version?

Or they should really be moved into a separate folder/package and then can clearly document this is an add-on not available for UI5 serve but then there should also be no hard references from the sandbox that is part of the UI5 serve packages. Just my opinion to keep the design clean, aligned and well documented.

RandomByte commented 3 years ago

Thank you for your input. I think I get your point. However, we can't easily remove resources from the CDN since developers might rely on their availability.

What we can do, and actually did in this case, is to leave them out from new delivery channels like npm and wait for feedback like this.

From my (limited) understanding, the responsible team plans to rework the sandbox and related demo apps in the long term.

You are right, I think we should check on the default behavior of sandbox.js always requesting fioriSandboxConfig.json. Thinking about this, maybe it makes more sense to replace fioriSandboxConfig.json with an npm-specific version during publishing.