RIAEvangelist / node-dominos-pizza-api

This is a node.js wrapper for the dominos pizza apis
MIT License
514 stars 126 forks source link

commonJS needs `./` in package.json exports field #100

Closed PatrickAlphaC closed 3 years ago

PatrickAlphaC commented 3 years ago

Can we use this with like an express server? I’m having a hard time

RIAEvangelist commented 3 years ago

Yes. This is written in node js. ES6 is one way of writing nodejs. You can import it as an ESM or require it as a commonJS module which is what I think you mean by asking if this will work in nodeJS.

I wish I could offer more help, but your question is a bit thin on details. If you would like to create a public repo and ask specific questions related to that code, I would be happy to help.

Best.

PatrickAlphaC commented 3 years ago

Thanks for helping out! And yes, sorry, I confused nodejs with commonJS

Whenever I try to require in a commonJS like so:

const dominos = require('dominos')

I get this error:

internal/modules/cjs/loader.js:430
      throw e;
      ^

Error [ERR_INVALID_PACKAGE_TARGET]: Invalid "exports" main target "index.js" defined in the package config /Users/patrick/code/vrf_pizza/pizza_cl_ea/node_modules/dominos/package.json; targets must start with "./"
    at throwInvalidPackageTarget (internal/modules/esm/resolve.js:304:9)
    at resolvePackageTargetString (internal/modules/esm/resolve.js:332:5)
    at resolvePackageTarget (internal/modules/esm/resolve.js:369:12)
    at packageExportsResolve (internal/modules/esm/resolve.js:470:22)
    at resolveExports (internal/modules/cjs/loader.js:424:36)
    at Function.Module._findPath (internal/modules/cjs/loader.js:464:31)
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:802:27)
    at Function.Module._load (internal/modules/cjs/loader.js:667:27)
    at Module.require (internal/modules/cjs/loader.js:887:19)
    at require (internal/modules/cjs/helpers.js:74:18) {
  code: 'ERR_INVALID_PACKAGE_TARGET'
}

I've tried with node 12 and 14.

PatrickAlphaC commented 3 years ago

In addition, if I change the exports in package.json of the node module to ./index.js I get:

internal/modules/cjs/loader.js:1080
      throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
      ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/patrick/code/vrf_pizza/pizza_cl_ea/node_modules/dominos/index.js
require() of ES modules is not supported.
require() of /Users/patrick/code/vrf_pizza/pizza_cl_ea/node_modules/dominos/index.js from /Users/patrick/code/vrf_pizza/pizza_cl_ea/order_pizza_revamped.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/patrick/code/vrf_pizza/pizza_cl_ea/node_modules/dominos/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1080:13)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/Users/patrick/code/vrf_pizza/pizza_cl_ea/order_pizza_revamped.js:2:13)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14) {
  code: 'ERR_REQUIRE_ESM'
}
PatrickAlphaC commented 3 years ago

Then, when attempting the recommended solutions here, I get a few different issues. Not sure where to go next.

PatrickAlphaC commented 3 years ago

Can we reopen and rename the issue if this makes sense? @RIAEvangelist

RIAEvangelist commented 3 years ago

The error you have gives the clue that is needed Must use import to load ES Module this is a commonJS error, it is not suggesting you do :

import dominos from 'dominos'

it is suggesting you do

dominos=import('dominos');

However, I made a quick project to test this, and indeed I need to add './' to the exports field in package.json

Tests are running now and I will publish once uploaded and tagged

PatrickAlphaC commented 3 years ago

Awesome! Thanks so much! Wondering if there could be an example using commonJS. Even with that tip, I'm still a little off. But looks like adding the ./index.js to the exports is good:

const dom = import('dominos')

const customer = new dom.Customer(
    {
        address: '123',
        firstName: 'first',
        lastName: 'last',
        phone: '###',
        email: '###'
    }
)

Error:

/Users/patrick/code/vrf_pizza/pizza_cl_ea/order_pizza_revamped.js:4
const customer = new dom.Customer(
                 ^

TypeError: dom.Customer is not a constructor
at Object.<anonymous> (/Users/patrick/code/vrf_pizza/pizza_cl_ea/order_pizza_revamped.js:4:18)
    at Module._compile (internal/modules/cjs/loader.js:1137:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
    at Module.load (internal/modules/cjs/loader.js:985:32)
    at Function.Module._load (internal/modules/cjs/loader.js:878:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47
RIAEvangelist commented 3 years ago

fixed in version 3.0.1

RIAEvangelist commented 3 years ago

Thanks @PatrickAlphaC

RIAEvangelist commented 3 years ago

If you want to add a hint for CommonJS usage in the readme, you will get credit for contributing.

ES6 example


import {Order,Customer,Item,Payment,NearbyStores} from 'dominos';

for commonjs should be changed to :


const {Order,Customer,Item,Payment,NearbyStores}=import('dominos');
RIAEvangelist commented 3 years ago

hmm... seem I need to put more effort into this... I just ran a test

const {Order,Customer,Item,Payment,NearbyStores}=import('dominos');

console.log({Order,Customer,Item,Payment,NearbyStores})

//returned

{
  Order: undefined,
  Customer: undefined,
  Item: undefined,
  Payment: undefined,
  NearbyStores: undefined
}
RIAEvangelist commented 3 years ago

OK, I got a working example of using this in commonjs. Personally I would recommend beginning to switch to ESM because it is more intuitive to import commonjs modules into ESM as it just works. Likely because ESM is the new standard and supports commonjs as legacy where commonjs is older and was designed before async ESM but this will get it working.

I'm going to add this to the docs.


let dominos;

(async () => {
    dominos=await import('dominos');
    //need to await dominos promise completion
    //because ES6 is async by nature
    start();
})()

//start app from a function instead of
//in the global scope
function start(){
    console.log(dominos.Order);
}

//outputs

[class Order extends DominosFormat]
RIAEvangelist commented 3 years ago

I am adding better docs for this now, but just to keep @PatrickAlphaC aware, here is the basis to let your code use the same code as the examples.


(async () => {
    const dominos=await import('dominos');

    //importing variables into the global like this just allows us to use the same code
    //from the ESM implementation for the commonJS implementation
    //This is the same as an ESM import of
//import {Address,NearbyStores,Store,Menu,Customer,Item,Image,Order,Payment,Tracking,urls,IsDominos} from 'dominos'

    Object.assign(global,dominos);

    //need to await dominos promise completion
    //because ES6 is async by nature
    start();
})()

function start(){
    //any example code would work as-is in here because the dominos guts are imported now.

    const n='\n';
    console.log(
        n,
        Address,n,
        NearbyStores,n,
        Store,n,
        Menu,n,
        Customer,n,
        Item,n,
        Image,n,
        Order,n,
        Payment,n,
        Tracking,n,
        urls,n,
        IsDominos,n
    );
}
RIAEvangelist commented 3 years ago

OK, I just pushed the changes to master.

See the short example as above in the main README.md

And the detailed example with a full order in the CommonJS.md

PatrickAlphaC commented 3 years ago

THANK YOU!!! THIS WORKS!!!!