Optimizer should process NPM packages

QwikDev / qwik

Instant-loading web apps, without effort

https://qwik.dev

MIT License

20.86k stars 1.31k forks source link

Optimizer should process NPM packages #483

Closed mhevery closed 2 years ago

mhevery commented 2 years ago

[ ] Inlined mode generates componentQrl($(() => {}, "FRIENFLYname")
[ ] Friendly names can not have underscores.
[ ] Dependencies are compiled with inlined strategy
[ ] We need a new scoping option (string) for libraries, using the package.name
[ ] Goal is for Qwikcity (used by npm create qwik) to use ___$() functions

PatrickJS commented 2 years ago

I added my comments in discussion: [Publishing a component library with Qwik]

It might make sense to create a standard packager like ng-packagr. I can work on the standard qwik packager if we agree on the specs later. For now a simple version of just making Optimizer look in node_modules should be good for now

kylecordes commented 2 years ago

There is a lot of merit in "just" processing Qwik components normally even if they come from node_modules - it avoids version-skew issues that will have to be dealt with if packages bundle up optimizer output. Might get a long way with this approach.

PatrickJS commented 2 years ago

In a way we modify the node_modules files anyways when we "tree shake" which is a performance step. The only way to go the next level for performance is to have more control over the packages and ideally the src of those packages.

Even though we may not want to add another step for package authors the truth is no one knows how to make a package for npm anymore (how to export, universal?, es5, commonjs, .mjs/.cjs?) so each framework must provide a package standard for their ecosystem.

We may not need a qwik-packager but say these are the requirements for qwik to read the components in node_modules correctly.

"exports": {
  "import": "./main.js",
  "require": "./main.cjs"
  "qwik": "./qwik.js"
},

"exports": {
  "import": "./qwik.js",
  "require": "./qwik.cjs"
},

mhevery commented 2 years ago

We are very much aware that it would be best if we did not have to do anything here (to @kylecordes point) However we don't think that is possible. So the goal is to have an absolute minimum transform.

manucorporat commented 2 years ago

Very close to get merged: https://github.com/BuilderIO/qwik/pull/628

manucorporat commented 2 years ago

Done