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

not available in npm #2

Closed
hopeng opened this issue Aug 1, 2022 · 15 comments
Closed

not available in npm #2

hopeng opened this issue Aug 1, 2022 · 15 comments

Comments

@hopeng
Copy link

hopeng commented Aug 1, 2022

Hi,

This tool looked fantastic! But why is it not available for installation from npm?

@robtweed
Copy link
Owner

robtweed commented Aug 1, 2022

Thanks! Glad you like it. I'd been holding off from adding DPP (and its dependency QOper8) to NPM as they're both designed for use in a browser only, but I do realise that a lot of people build their client-side browser code using NPM and WebPack.

I've now added DPP and QOper8 to NPM. To avoid name clashes in NPM, they are named dpp-db and qoper8-ww respectively. As I'm not an expert on building front-end code using NPM, I'm not sure if what I've put in the package.json files for these two modules will be sufficient. Perhaps you (or others) could suggest any appropriate changes if they are needed for NPM?

Let me know how it goes!

@hopeng
Copy link
Author

hopeng commented Aug 1, 2022

Thank for the quick response! It'll be great to add types declaration to the package for typescript webapps to work properly and not having to use dynamic import() (I'm using angular). I tried dts-gen trying to generate the *.d.ts for the two modules but no luck. Looks like it needs to be defined manually.

One issue I found is my webapp failed to generate browser application bundles with webpack, showing dynamic import() error:

external "https://robtweed.github.io/QOper8/src/QOper8.min.js" - Error: The target environment doesn't support dynamic import() syntax so it's not
 possible to use external type 'module' within a script
Did you mean to build a EcmaScript Module ('output.module: true')?

with just this line in my code

    // @ts-ignore
    const {DPP} = await import("node_modules/dpp-db/src/dpp.js")

I'm still trying to figure out why, but looks like the library has hard coded default reference to QOper8.min.js and webpack is trying to reach out to the referenced link but failed at dynamic import()?

@robtweed
Copy link
Owner

robtweed commented Aug 1, 2022

Not being a Typescript developer, I'm afraid I don't know what would be needed to address the first issue. If you (or someone else) can add a pull request to provide what's needed for Typescript users, I'd be happy to add it to the repo (provided it doesn't break anything anywhere else).

As noted in the documentation, DPP has a dependency on QOper8 which NPM should also have loaded for you when you installed DPP from NPM. However, DPP has no way of knowing where your local copy of QOper8 will be, so, unless you explicitly tell it where to find it, it will default to dynamically importing a copy from the Github repository, which is what has happened in your case.

For manually-written front-end code this isn't a problem, but I can imagine it will cause issues if you're using a build chain eg using Node.js and WebPack.

You'll see in the documentation how to tell DPP where your local copy of QOper8 resides. Notice, however, that you need to also help QOper8's WebWorker Loader module so it knows where your local copies of the DPP IndexedDB handlers reside (again, if you don't tell it, it will use copies from the DPP Github repo).

If you're doing it manually, you'd do something like this, so you'd need to adapt it for your build scenario:

        const {DPP} = await import('/dpp/dpp.min.js');
        const {QOper8} = await import('/qoper8/QOper8.min.js');

        let storeName = 'myObjCopy';
        let idb_name = 'MY-DB';

        let dpp = new DPP({
          storeName: storeName,
          idb_name: idb_name,
          QOper8: QOper8,
          qOptions: {
            workerLoaderPath: '/qoper8/',
            handlerPath: '/dpp/idb_handlers/'
          }
        });

Anyway I hope this helps move you forward. BTW you may find you need to rename the modules with a .mjs file extension since Node.js v18 now expects module files to have this extension (unlike the browser) - if so, I guess I can add copies of such module files with an .mjs extension into the repository. Let me know what you think.

@hopeng
Copy link
Author

hopeng commented Aug 2, 2022

I have to comment out these two lines from my node_modules/ source for angular webpack to work with your provided code.
https://github.com/robtweed/DPP/blob/master/src/dpp.js#L261-L262

Otherwise webpack failed at this line:
const {DPP} = await import('/dpp/dpp.min.js');
with the "doesn't support dynamic import() syntax" error, possibly due to the build tries to resolve all possible external URLs during packaging, even the new DPP() instance specify alternative QOper8 object.

Is it possible to have a 'local' way to set default QOper8 instead of dynamic import() external URL? Since your dpp-db package depends on qoper8-ww anyway, you'll assume the client app will have qoper8-ww/src/Qoper8.min.js inside their node_modules/ dir already.

@robtweed
Copy link
Owner

robtweed commented Aug 2, 2022

OK I've updated the package.json files for qoper8-ww and dpp-db to fix a couple of Node.js-related issues. So you first install DPP using:

npm install dpp-db

I've tested a script file using Node.js v18. It has to be a module, so I named it test.mjs

It contained the following:

import {QOper8} from "qoper8-ww";
import {DPP} from "dpp-db";

let dpp = new DPP({
  storeName: "test",
  QOper8: QOper8,
  qOptions: {
    workerLoaderPath: '/qoper8/',
    handlerPath: '/dpp/idb_handlers/'
  }
});

This loaded DPP and QOper8 successfully from node_modules and instantiated DPP. Of course I can't run it because this is Node.js and not a browser. However, at least this demonstrates that all the required resources can now be loaded within Node.js.

So this means you can now import both DPP and QOper8 as shown, and QOper8 can be passed into the DPP constructor - that will bypass the lines you commented out, so there's no need for you to comment them out.

The remaining issue is the workerLoaderPath and the handlerPath values. You need to understand that these are used as follows:

  • QOper8 needs to start its WebWorker in the standard way, which is via a URL. You can't start a WebWorker in any other way, so DPP needs enough information to be able to construct the URL that the browser will then use to fetch, in this case, the QOper8Worker.js file. If you're happy for the browser to fetch a copy from the Github Repo, you can leave out the qOptions.workerLoaderPath property from the DPP constructor and it will start the WebWorker process just fine. However, if you want to load a local copy of QOper8Worker.js from your web server, then a) You'll need to have put a copy of the QOper8Worker.js file on the web server and b) provide the web server alias path to it as the value for qOptions.workerLoaderPath. In other words, there's no way to pre-pack the QOper8Worker.js file into the bundle you're generating with WebPack and get it to be used at run-time in the browser.

  • Within the QOper8 WebWorker process, it needs to dynamically load, when needed, the DPP handler JS files that look after indexedDB. The way this is done in a WebWorker is via the importScripts() command which also needs a URL for the imported script file. So the browser will then fetch the script file using that URL when it's needed - again there's no way to pre-load it from a WebPack-generated bundle. Once again, if you're happy for the QOper8 WebWorker to load the DPP handler scripts from the Github repository, then you can leave out the qOptions.handlerPath property and it will work just fine for you. However, if you want to load the DPP handler scripts from your web server, then a) You'll need to have put a copy of DPP's idb_handlers folder on your web server and b) provide the web server alias path to that folder as the value for qOptions.handlerPath

If you do all these things, you'll avoid and bypass all of the dynamic module imports otherwise used by DPP

Let me know how you get on!

@robtweed
Copy link
Owner

robtweed commented Aug 2, 2022

An update - I've figured out how to package up the QOper8 Worker Script and also the DPP indexedDB handler scripts within the main body of the QOper8.js and dpp.js code. So you can now forget about the qOptions complications.

The following should now be sufficient when working with NPM:

import {QOper8} from "qoper8-ww";
import {DPP} from "dpp-db";

let dpp = new DPP({
  storeName: "test",
  QOper8: QOper8
});

I think this simplifies things a lot!

@hopeng
Copy link
Author

hopeng commented Aug 2, 2022

Thanks for the quick update! It does simplifies a lot. Is it possible to replace the dynamic import with static import? This https://github.com/robtweed/DPP/blob/master/src/dpp.js#L261-L262 is still breaking webpack with the same error.

But webpack works when I put this at the top of dpp.js
import {QOper8} from "qoper8-ww";

And change the create(option) method to:

  static async create(options) {
    if (!options.QOper8) {
      options.QOper8 = QOper8;
    }
    let dpp = new DPP(options);
    return dpp;
  }

@robtweed
Copy link
Owner

robtweed commented Aug 3, 2022

So the problem is that the dpp.js module must be able to be loaded dynamically into another browser module if the front-end code is hand-crafted. In that scenario, there's no way for it to know where to load the QOper8 module from. So it can't simply use

import {QOper8} from "qoper8-ww";

because that only works if you've loaded QOper8 via dpp-db using NPM. Hence its fall-back on a dynamic import (which apparently, from what you're saying, WebPack doesn't like).

That having been said, it's not clear to me why you can't use my suggestion: you already must be doing:

import {DPP} from "dpp-db"

somewhere before calling DPP's constructor, so why not just also import QOper8 at the same time:

 import {DPP} from "dpp-db"
 import {QOper8} from "qoper8-ww"

If you installed dpp-db using NPM, then qoper8-ww will have also been installed (since it's declared in DPP's package.json as a dependency), so you can guarantee the two lines above will work.

Then you can just do this:

let dpp = new DPP({
  storeName: "test",
  QOper8: QOper8
});

ie you call the DPP constructor directly and pass the QOper8 class into it.

Can you let me know if/why this won't be possible or work for you?

Many thanks
Rob

@hopeng
Copy link
Author

hopeng commented Aug 3, 2022

Hi Rob,

I am doing what you suggest in my angular code and it actually works from local machine if I don't creates the browser deployment bundle.

 import {DPP} from "dpp-db"
 import {QOper8} from "qoper8-ww"
let dpp = new DPP({
  storeName: "test",
  QOper8: QOper8
});

I managed to reproduce the error with a minimum webpack demo. The issue is with webpack will create code bundle that merge and minify my code with all dependencies code into a single main.js file. It collects the dpp-db lib source code and resolves your dynamic import() calls as soon as dpp-db is imported, even before the line new DPP() was called:
https://stackblitz.com/edit/github-gg7fhv?file=src%2Findex.js%3AL18

if you run:
npm install && npm run build

You'll see the problem I described. But if you comment out the DPP related code the build will work.

And if I replace dynamic import with static import and uncomment the DPP code, it will also work because webpack 'resolved' the imported QOper8.js and put everything into a minified bundle.

@robtweed
Copy link
Owner

robtweed commented Aug 3, 2022

OK so it appears that WebPack doesn't like to see any references to dynamic imports, even if it won't actually invoke them at run-time. Similarly, Node.js doesn't like "import from..." commands at anything other than the top level. So it's an interesting puzzle how to satisfy the requirements and limitations of both local builds and WebPack builds.

With a bit of thinking, I believe I've figured out a way to have it available for both local manual front-end builds and WebPack builds. It will involve a different way of starting DPP, but actually it will be simpler for everyone in the end. Just a bit complex internally, especially for the derived databases I've created (Key/Value and List). So I won't get it all sorted out and documented until tomorrow, but I think I have the important parts ready. Just needs some finishing off and testing before committing it to the repo.

Your feedback has been very helpful in figuring this out!
Regards
Rob

@robtweed
Copy link
Owner

robtweed commented Aug 4, 2022

OK see if the latest update does the trick for you.

First, install the latest version of dpp-dbb using NPM

Then you just need to do the following:

import {createDPP} from "dpp-db/node";

let DPP = createDPP({
  storeName: 'myStore'
});

let local_obj = await DPP.start();

The new Node.js-based createDPP module will load DPP and QOper8 from your local DPP installation. DPP no longer contains any dynamic imports within its code, so I believe it should now work. createDPP ensures that DPP has everything it needs to start up.

Let me know how you get on.

Take a look at the KV.md and LIST.md documents - there are instructions on how to use them in a WebPack/Node build (assuming I've got everything properly in place now!)

Rob

@hopeng
Copy link
Author

hopeng commented Aug 5, 2022

unfortunately it's still an issue. You can see that from my stackblitz example by upgrading dpp-db to 2.3.0. I'm still seeing the dynamic import() error.

again if I checkout the stackblitz demo project, run npm install, comment out these two lines from the local node_modules/dpp-db/src/dpp.js and then run 'npm run build' again. It will work.
https://github.com/robtweed/DPP/blob/master/src/dpp.js#L277-L278

@robtweed
Copy link
Owner

robtweed commented Aug 5, 2022

Apologies - my stupid mistake: I'd not removed the very chunk of code that was causing the issue in dpp.js! I've pushed a new version (2.4) to NPM - I think that should fix it!

@hopeng
Copy link
Author

hopeng commented Aug 6, 2022

works like a charm! thank you very much!

@robtweed
Copy link
Owner

robtweed commented Aug 6, 2022

Great news! Thanks for your patience and assistance on this. I'll close this issue now.

@robtweed robtweed closed this as completed Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants