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

[ui5-tooling-modules] includeAssets not respected in middleware #1074

Closed
HymanZHAN opened this issue Sep 10, 2024 · 5 comments
Closed

[ui5-tooling-modules] includeAssets not respected in middleware #1074

HymanZHAN opened this issue Sep 10, 2024 · 5 comments
Assignees
Labels
author action Waiting for feedback by issue author enhancement New feature or request

Comments

@HymanZHAN
Copy link

HymanZHAN commented Sep 10, 2024

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

When using 3rd party libraries, I'd like to include other assets like CSS while developing locally. However, it seems that includeAssets doesn't apply to ui5-tooling-modules-middleware.

Describe the solution you'd like

I want my CSS to be copied and served from /resources when developing locally using @ui5/cli.

Describe alternatives you've considered

Load the stylesheets from CDN.

Additional context

I wish the configurations like includeAssets and addToNamesapce are aligned between serve and build. Otherwise, how are we supposed to verify/develop things locally?

@petermuessig
Copy link
Member

@HymanZHAN

Rgd. includeAssets: the assets can be accessed directly as the middleware can directly access the assets of the node modules. Therefore this is not needed. So /resources/mynpmpkg/css/my.css is always accessible via middleware but for the task you need to explicitly declare it if it is not used via sap.ui.require.toUrl.

Rgd. addToNamespace: I thought about this already but this is means a complete change of the implementation and it would require to rewrite the sources already during the serve. It makes it harder in several places ... But I can think again about this.

@petermuessig petermuessig added enhancement New feature or request blocked Waiting for basic requirements to be fulfilled labels Sep 10, 2024
@petermuessig petermuessig self-assigned this Sep 10, 2024
@petermuessig petermuessig changed the title includeAssets not respected in ui5-tooling-modules-middleware [ui5-tooling-modules] includeAssets not respected in middleware Sep 10, 2024
@petermuessig petermuessig added author action Waiting for feedback by issue author and removed blocked Waiting for basic requirements to be fulfilled labels Sep 10, 2024
@petermuessig
Copy link
Member

@HymanZHAN let's use the #1049 to track the feature request for the addToNamespace option being considered in the middleware. As said, this might be a bigger change and will also slow down the middleware due to the necessary rewrite of the resources.

If you are OK with my answer regarding includeAssets, we can close the ticket here. As the middleware can just access them via the resources path there is no feature needed for that. The only thing what I can imagine of for includeAssets is to restrict the access to any other asset not being mentioned here.

@HymanZHAN
Copy link
Author

HymanZHAN commented Sep 11, 2024

Got it. Wasn't aware that the middleware has full access to node_modules, which is pretty cool 😁 I will experiment a bit more with accessing css resources and if it worked out, I will come back and close the issue.

I'd also like to learn about how the middleware decides which version of .js file to use if there are multiple ones. For example, for sinon there are sinon.js, sinon0esm.js and sinon-no-sourcemaps.cjs in its pkg folder. How does the middleware determine which one gets transpiled?

@petermuessig
Copy link
Member

@HymanZHAN - you can take a look into the resolveModule function

The logic is as follows: resolveModulePath
1.) one of the properties "browser", "import", "require", "default" in package.json > exports
2.) one of the properties "browser", "import", "require", "default" in package.json
3.) one of the properties: "browser", "module", "main" in package.json > exports
4.) one of the properties: "browser", "module", "main" in package.json

Once a value is found in the one of the properties above it loads either: resolveNodeModule
1.) .js
2.) .cjs
3.) .mjs

This is on a very coarse view explained how the module resolution works. I tried to imitate the node lookup does but I needed to enhance it.

The final truth is in the code... 😉

@petermuessig
Copy link
Member

@HymanZHAN as mentioned above, we use #1049 to track the feature request for the addToNamespace option and close the issue here fore the other question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author action Waiting for feedback by issue author enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants