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

Shims no longer bundled after migration to v3 #807

Closed
dfenerski opened this issue Mar 23, 2023 · 4 comments · Fixed by SAP/ui5-project#612
Closed

Shims no longer bundled after migration to v3 #807

dfenerski opened this issue Mar 23, 2023 · 4 comments · Fixed by SAP/ui5-project#612
Assignees
Labels
bug Something isn't working

Comments

@dfenerski
Copy link

dfenerski commented Mar 23, 2023

Hi, I am creating this issue because the 'get a slack invite' website seems to crash on email submission..
So - I've followed the migration guide and attempted to build an app with a dedicated library using the -a flag. Earlier this used to mount all shims under the /resources dist directory, but now only some? shims are mounted. I cannot seem to find any info regarding shims & this behavior in the new tooling version. What am I missing?

Expected Behavior

Shims in libraries should be in the dist of the consuming app

Current Behavior

None or only some shims are bundled in the dist

Steps to Reproduce the Issue

{...}

Context

  • UI5 Module Version (output of ui5 --version when using the CLI): 3.0.5
  • Node.js Version: 16.9.1
  • npm Version: 8.19.3
  • OS/Platform: Win11
  • Browser (if relevant): not relevant
  • Other information regarding your environment (optional): nothing
@dfenerski dfenerski added bug Something isn't working needs triage Needs to be investigated and confirmed as a valid issue that is not a duplicate labels Mar 23, 2023
@dfenerski
Copy link
Author

dfenerski commented Mar 23, 2023

Follow-up: ui5 tree lists the shims that do get bundled. So if it is in the dep graph, it gets properly bundled.

ui5 build --excludeTask=* --includeTask="problematicLib" --verbose yielded the following line for a shim that does not get bundled: Module rxjs neither provides a project nor an extension. Skipping dependency resolution as well as Warning - Dependency rxjs is available at multiple paths:

Log statement is here

The app has depDependency to rxjs so that only the TS types are available. The lib has dependency to rxjs so that it gets installed, shimmed and bundled there. This is the project logic.

If my understanding is correct, the first occurrence of rxjs is in the root, and therefore the tooling prefers it, but the shim is defined in the library, so it does not get copied around as multiple apps consume the lib. Is there a way to adjust this behavior?

@dfenerski
Copy link
Author

Another follow-up: disabling devDependency analysis here and here solved my problem. I think this confirms the issue as I understand it in the comment above - the tooling refuses to bundle when a shim is duplicated twice in a consumer app and in an underlying lib. However I think the use case is legit - the lib shims & marks as dependency once; any consumer app specifies the lib as dep and the shims as devDependency for type usage only.

Am I wrong? Can this behavior be tweaked?

Additional question, perhaps a sillier one - why are depDependencies even bundled & shipped to the browser? Isn't their point to be dev only ?

Any information on those questions is appreciated. Thanks in advance.

@matz3 matz3 self-assigned this Apr 5, 2023
@matz3 matz3 removed their assignment Apr 14, 2023
@d3xter666 d3xter666 self-assigned this May 11, 2023
@d3xter666
Copy link
Contributor

Hi @dfenerski ,

Would be able to elaborate a little bit more?

What I understand so far is that you have couple projects an app and some library that is dependency of that app.
The rxjs is a dependency of the library, but in the app is used only when developing (devDependencies), so you'd have TS definitions available for it. And you'd end up with a structure like this:

App structure

app/
  \_ node_modules/
    \_ library/
      \_ node_modules/
        \_ rxjs/

app's package.json

{
  "name": "app",
  ...
  "dependencies": {
    "lib": "^X.Y.Z"
  },
  "devDependencies": {
    "rxjs": "^X.Y.Z"
  }
}

library's package.json

{
  "name": "lib",
  ...  
  "dependencies": {
    "rxjs": "^X.Y.Z"
  }
}

library's shim for rxjs (eventually) in ui5.yaml

specVersion: "3.0"
kind: extension
type: project-shim
metadata:
  name: rxjs
shims:
  configurations:
    rxjs:
      specVersion: "3.0"
      type: module
      metadata:
        name: rxjs
      resources:
        configuration:
          paths:
            /resources/: ""

If I understand correctly your complaint, you'd expect that dependencies for the library would be in the dist/ folder of the app when buliding the app.

Did I understand it correctly?

@d3xter666 d3xter666 added the information required Further information is required label May 11, 2023
@dfenerski
Copy link
Author

Thanks for reaching out @d3xter666, that's pretty much it.

What essentially happens is that app reuses a dependency of library instead of installing it on its own, so that node_modules / the dist bundle are slimmer, the shim is not redefined & any other client apps app1, app2, etc can easily import it (shared shim). As the project runs on TS, type definitions for this reused dependency are installed as devDependency only.

I.e app uses rxjs also at runtime & not only during development time: the library uses sap.ui.loader.config to provide import shim which points sap.ui.define(['rxjs']) to resources/library/shimmed_modules/rxjs.

Therefore:

  • rxjs is installed, shimmed & bundled only once
  • any consumer app of library automatically has the shim available & can use simple imports without having to redefine it or bundle the dependency again

If we were to stick with the default dependencies-only bundling, each consumer app would have to install rxjs, define its own local version of the shim (the local install will map the OSS to local virtual path) & only then be able to sap.ui.define(['rxjs']).

Looking for a way to standardize/automate this process in a multi-app/lib context led me to this issue, it could very well be that I've missed something in the process.

RandomByte added a commit to SAP/ui5-project that referenced this issue May 17, 2023
If a visited module did not resolve to any specification (project or
extensions), try recreating the module when visiting it again from a
different parent.

Resolves SAP/ui5-tooling#807

This is an alternative to #611

This change might have a greater impact on performance since er might
recreate modules that are not relevant to UI5 Tooling more than once if
they are listed multiple times in the dependency tree.
Before this change, such modules where only visited once.
RandomByte added a commit to SAP/ui5-project that referenced this issue May 22, 2023
If a visited module did not resolve to any specification (project or
extensions), try recreating the module when visiting it again from a
different parent.

Resolves SAP/ui5-tooling#807

This is an alternative to #611

This change might have a greater impact on performance since er might
recreate modules that are not relevant to UI5 Tooling more than once if
they are listed multiple times in the dependency tree.
Before this change, such modules where only visited once.

---------

Co-authored-by: Yavor Ivanov <yavor.ivanov@sap.com>
@flovogt flovogt removed information required Further information is required needs triage Needs to be investigated and confirmed as a valid issue that is not a duplicate labels May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants