Alternative ways of exposing pnp to the end user (that doesn't break the end-user consumption of lib)

[pbjorklund]

Design goal

First I would like to see if everyone is aware of the design goal of project of making sure that consumption should be done by including the library and then being able to start using it directly through the pnp object. I myself did not realise this was set in stone which led to some confusion in the discussions that followed.

Usage as shown in server-root/index.html

<script type="text/javascript">

       require(['scripts/pnp'], function(pnp) {        

           alert("our random string: " + pnp.util.getRandomString(5));         
       });
    </script>

This should be documented somewhere seeing as it's not going to be possible to change this signature after release.

Creating the pnp object

The problem is that the transpilation gives us an object where the code is accessible through pnp.defaults instead of pnp directly as the transpilation adds some metadata to the object exposed. (The replace below shows what I mean)

The current approach around this is to rewrite the transpiled code by adding the following to the package.js build task (line 84 and 102)

 .pipe(replace(/Object\.defineProperty\(exports, "__esModule", \{ value: true \}\);/ig, ""))
 .pipe(replace(/exports.default = PnP;/ig, "return PnP;"))

An alternative approach

I noticed that exposing the properties of the PnP class directly through export const sharepoint = new SharePoint(); would generate a pnp object that is consumable without rewriting the transpiled code with replace.

Two approaches that generate the same end-user result

Exporting a class and using .replace() to rewrite the source

export default class PnP {
    public static sharepoint = new SharePoint();
    public static sp = new Rest();
}

Exporting constants without the use of a class in pnp.ts and with no .replace()

export const sharepoint = new SharePoint();
export const sp = new Rest();

The above 2 examples will both generate an object that we can do pnp.web.lists on in the browser.

Lib output

The class example will generate this code in /lib

export default class PnP {
}
/**
 * Utility methods
 */
PnP.util = Util;
/**
 * The full SharePoint library
 */
PnP.sharepoint = new SharePoint();

The const example will generate this code in /lib

export const sharepoint = new SharePoint();
export const sp = new Rest();

More background

Related discussions that led to this issue can be found in #78 and #80.

[patrick-rodgers]

I have just added a design goals page to the wiki, so we document things. Only so much time in the day.

Have you tested this second format when importing into a new TS project from the npm package? I.e. does this still work:

import pnp from "sp-pnp-js";

This was one of the "big" problems that started this entire conversation so want to ensure we maintain that functionality as well.

[pbjorklund]

Good job on the design docs!

No, you would have to import pnp as import * as pnp from "sp-pnp-js" seeing as there is no class being exported.

If we add a

export default {
    log: sp,
    sharepoint: sharepoint,
    sp: sp,
    storage: storage,
    util: util,
    // configuration = configuration;
}

Then we can import it in serverside land. (Yeah, I accidently set log to sp, ignore that in the screenshot :))

But we get duplication in browserland (consumption works with pnp.log.whatever tho).

The complete pnp.ts is then

"use strict";

import { Util } from "./utils/Util";
import { SharePoint } from "./SharePoint/SharePoint";
import { PnPClientStorage } from "./utils/Storage";
// import * as Configuration from "./configuration/configuration";
import { Logger } from "./utils/logging";
import { Rest } from "./SharePoint/Rest/rest";

/**
 * Root class of the Patterns and Practices namespace, provides an entry point to the library
 */
/**
 * Utility methods
 */
export const util = Util;

/**
 * The full SharePoint library
 */
export const sharepoint = new SharePoint();

/**
 * Provides easy access to the REST interface
 */
export const sp = new Rest();

/**
 * Provides access to local and session storage
 */
export const storage: PnPClientStorage = new PnPClientStorage();

/**
 * Global configuration instance to which providers can be added
 */
// export const configuration = Configuration;

/**
 * Global logging instance to which subscribers can be registered and messages written
 */
export const log = Logger;

export default {
    log: log,
    sharepoint: sharepoint,
    sp: sp,
    storage: storage,
    util: util,
    // configuration = configuration;
}

[alex-randall]

@pbjorklund Excellent evaluation, testing, research on this so far!
@patrick-rodgers Awesome about the design goals wiki page. Love it so far!

[patrick-rodgers]

My vote is to close this issue and move on. The string replacements work, and we don't have to duplicate everything in pnp.ts. This is getting around a specific limitation in how TypeScript builds the libraries, and was only an issue because the idea that we would use the syntax:
import pnp = require("sp-pnp-js");
was considered unacceptable. I don't have a problem with that syntax but have tried to accommodate the feedback. Which led down this rabbit hole. Perhaps one day we can do something different but for now unless there is a very compelling reason let's move on.

[pbjorklund]

If you say so. Your library, your call.

[pbjorklund]

The duplication in the dist above also allows people to build something by refing NPM package without bundling in their own dist of pnp. Now it will transpile to for instance pnp.default.sp which does not exist on the pnp.js in the dist folder.

(Taken from discussion on gitter with @derek-smith this weekend)

[patrick-rodgers]

Just read through gitter. Not sure I understand 100% - so including the .default enables what behavior? If it helps something then we should include it in our library.

[pbjorklund]

So here is the thinking.

Let's say I start a new project and use webpack to bundle my app.

I install the sp-pnp-js library and import it into my solution to get the intellisense.

Now I don't want to bundle the NPM packages /lib into my solution (with something like webpack) since I already have it globally accessible on pages in SharePoint outside of my own app-code so I rely on pnp already being loaded. For instance via a script tag pointing to the /dist (or in future bower) package of pnp.js on my cdn.

Calls made in my transpiled source will point to pnp.default which does not exist in the /dist version of pnp.js.

index.ts

import pnp from "sp-pnp-js";

pnp.sp.web.get().then((web) => {
    console.log(web.title);
})

index.js commonjs

"use strict";
var sp_pnp_js_1 = require("sp-pnp-js");
sp_pnp_js_1["default"].sp.web.get().then(function (web) {
    console.log(web.title);
});

index.js umd

(function (factory) {
    if (typeof module === 'object' && typeof module.exports === 'object') {
        var v = factory(require, exports); if (v !== undefined) module.exports = v;
    }
    else if (typeof define === 'function' && define.amd) {
        define(["require", "exports", "sp-pnp-js"], factory);
    }
})(function (require, exports) {
    "use strict";
    var sp_pnp_js_1 = require("sp-pnp-js");
    sp_pnp_js_1.default.sp.web.get().then(function (web) {
        console.log(web.title);
    });
})

[patrick-rodgers]

Ah, so other things are thinking the default will be there because that is how the TS code is structured. Got it. So the supported TS way to do this is what we did previously with "export =", but we can't because folks want to use the newer import syntax.

I am playing a bit more with packaging today to see if I can get the size down some more, I'll take another look at this. Likely we'll just have to duplicate code in pnp.ts to cover all the bases.

[patrick-rodgers]

Completed here #110