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

project reports cant find package for a package that is not referenced at all #1832

Closed
vladmandic opened this issue Apr 14, 2022 · 10 comments
Closed

Comments

@vladmandic
Copy link

vladmandic commented Apr 14, 2022

project reports cant find package for a package that is not referenced at all

simple reproduction: https://stackblitz.com/edit/typescript-blff19

project as-is works fine, but uncommenting two lines that import and access @tensorflow/tfjs-backend-cpu results in build error:

Can't find package:@tensorflow/tfjs-node

but @tensorflow/tfjs-backend-cpu does not reference @tensorflow/tfjs-node anywhere
(and yes, @tensorflow/tfjs-node cannot be installed since its a nodejs module while this is a pure browser project)

the only mention of tfjs-node inside tfjs-backend-cpu module anywhere is inside string block:

backend_util.warn('\n============================\n' +
    'Hi there 👋. Looks like you are running TensorFlow.js in ' +
    'Node.js. To speed things up dramatically, install our node ' +
    'backend, which binds to TensorFlow C++, by running ' +
    'npm i @tensorflow/tfjs-node, ' +
    'or npm i @tensorflow/tfjs-node-gpu if you have CUDA. ' +
    'Then call require(\'@tensorflow/tfjs-node\'); (-gpu ' +
    'suffix for CUDA) at the start of your program. ' +
    'Visit https://github.com/tensorflow/tfjs-node for more details.' +
    '\n============================');

is it possible that stackblitz parser misbehaves and does not consider this a string and reports a dependency purely because of this?

also note that using other libraries such as @tensorflow/tfjs-backend-webgl that have a hard dependency on @tensorflow/tfjs-backend-cpu do not present an issue and are working just fine

and according to 3rd party user reports, all this used to work until january when some changes went live on stackblitz (according to some unconfirmed notices in old issues, thats when new module parser when live)?

note: this issue is also being tracked by @tensorflow team as a P0 issue - see tensorflow/tfjs#6315

@markwhitfeld
Copy link
Member

Thank you for reporting this issue. I am adding it to my list for investigation.

As an alternative to unblock you, you could move this code to a WebContainer node project (https://stackblitz.com/fork/node) which will work the same as a local node project. You will need to add a bit more code to "serve" the example app though.

@markwhitfeld markwhitfeld self-assigned this Apr 27, 2022
@vladmandic
Copy link
Author

@markwhitfeld unfortunately, suggested workaround is not an option as this is not about single project (even then moving it from browser to node is basically changing it completely) as tensorflow is used by hundreds of modules and all of them are impacted by this core issue - thus high severity impact.

@markwhitfeld
Copy link
Member

Ok, thank you for the additional context. If there is anything else that you can provide that may help to debug this, please add the info. I will take a look tomorrow.

@vladmandic
Copy link
Author

vladmandic commented Apr 27, 2022

in case you need to understand tensorflow module internal dependencies:

  • @tensorflow/tfjs-core core module itself without any platform dependent code (there are few others like that, -data, -layers, -converter)
  • @tensorflow/tfjs-backend-cpu math ops implemented in js, basically a fallback when optimized implementation is not present; without dependencies on platform-specific implementations (thus the issue here - it does not reference or need tfjs-node)
  • @tensorflow/tfjs-node core + cpu + bindings to tensorflow.so so it runs optimized for nodejs
  • @tensorflow/tfjs-backend-webgl core + cpu + optimized math in webgl for browser environments
  • @tensorflow/tfjs - bundle of core + data + layers + converter + cpu + webgl

@markwhitfeld
Copy link
Member

If you are able to create a bare minimal reproduction project, with the least amount of code and the smallest number of dependencies, it will do wonders for getting this resolved quickly... Plus it will become part of our internal acceptance test pack.🎉

@vladmandic
Copy link
Author

vladmandic commented Apr 27, 2022

i did that, link is in the original issue, copied here as well: https://stackblitz.com/edit/typescript-blff19
it works fine until you un-commented two lines (marked) and then fails with invalid dependencies.

@markwhitfeld
Copy link
Member

The root of this issue lies in a bug in SystemJS.
I have submitted a PR to fix it but in the mean time we will implement a localised patch for the SystemJS issue.

One critical piece of info I need from you though, is about which js file in your package should be used in the browser.
The package.json for @tensorflow/tfjs-backend-cpu exposes a few different files:

  "main": "dist/tf-backend-cpu.node.js",
  "jsdelivr": "dist/tf-backend-cpu.min.js",
  "unpkg": "dist/tf-backend-cpu.min.js",
  "types": "dist/index.d.ts",
  "jsnext:main": "dist/index.js",
  "module": "dist/index.js",

The normal default in StackBlitz is to use the main field but, with node in the name, I was wondering if it would be appropriate to use in a browser environment?

@vladmandic
Copy link
Author

thanks - seems my guess about string parsing was good.

anyhow, there is no real difference between tf-backend-cpu.node.js and tf-backend-cpu.min.js, its mostly artifact of the overall build process (it does matter for some other tf modules).

anyhow, typical behavior for most build environments is that if module field is present and module is required, then that is used while main is used as a fallback if module is not present or requested. e.g, import ... would load an entry from module while require ... would load entry from main regardless if its browser or node environment.

@markwhitfeld
Copy link
Member

markwhitfeld commented Apr 30, 2022

Thanks! Ok, that is great news, because we will be able to add a quicker fix for you that won't depend on that SystemJS PR.
Yeah, there is the ideal of how the package fields are handled, but unfortunately there is a large legacy of libraries that have done things in different ways. We plan to move to the ideal, but it will have to be in small incremental steps so that we do not break a large number of projects in one go.
There was a legacy change for d3 that preferred the unpkg field over anything else if it existed, but this is causing more and more issues with correctly configured libraries (like yours). We currently have an exception list for this rule but we will be changing this to an "opt-in" list soon instead for libraries that do not follow the ideal as part of the migration.
It just so happens that your main package doesn't have the issue because the text containing the require in the tf-backend-cpu.node.js file uses escaping (require(\'@tensorflow/tfjs-node\')) which happens to avoid this SystemJS bug.

@markwhitfeld
Copy link
Member

@vladmandic This issue should be resolved now. Apologies for the radio silence, my PC's SSD crashed last week!
Let me know if you have any issues.

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

No branches or pull requests

2 participants