Move provisioning to own package, split types and move Util to a class

[pbjorklund]

Edit: See comments below.

I was playing around with why PnP was loading as pnp.default and ended up trying out using a pnp.Core namespace and moving provisioning to pnp.Provisioning with separate dist files.

Now this is a major change, but it was easier to just do it then write a long issue for it.

Check it out and let me know what you think, I'm not even 100% sold myself.

Changed the served index.html to do this

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

        var core = new pnp.Core();
        alert("our random string: " + core.util.getRandomString(5));
       });

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

        var prov = new pnp.Provisioning();
        alert("Provisioning handler name: " + prov.core.handlers.ComposedLook.name);
       });

• refactored utils/util to class intead of collection of functions. SharePoint util is still collection of funcs

[pbjorklund]

Oh and a note, it's not actually 2 modules right now - I think everything is in both. It's still a WIP :)

Edit: Or did I fix that? Not sure. I'l update when I get back.

[patrick-rodgers]

This breaks one of the core goals of the project, being the ability to include pnp and begin using it directly. The entire library is the "core" so we want to be able to require(), function(pnp) { pnp.* }. For changes this big we really need some discussion in the future as you've obviously put in a lot of effort but I don't want to merge it because I feel it breaks one of the core use cases.

[pbjorklund]

I don't necessarily think that including lot's of code that someone might not want to use (i.e provisioning) every time should be a goal of the project. What I'm after (in the long run) is being able to select just the parts of the library that I need for the specific application that I'm building.

From my understanding it's only the way we expose the library that's the issue here? I guess we could expose an already newed Core() and that problem would go away.

The PR mainly splits core and provisioning into separate folders and built files.

It seems like pnp.defaults would be the way to expose the library. Every way I could think of exposing it directly as pnp gives me compilation errors. See microsoft/TypeScript#2719 which is related. If someone could enlighten me on what is going that would be great as I'm not deep into ES6 and frankly find the whole module handling pretty confusing.

It's worth discussing a change like this now though rather than later seeing as when it launches there will be no way to change anything without breaking backwards compability.

In my head not exposing a constructor is a bad thing. I would like to see for instance new Core(configurationObject) vs pnp.configuration = object.

Edit: And effort really shouldn't be a factor here, it just took less time to actually write the code then an issue detailing everything.

[mike-morr]

@patrick-rodgers I thought that entire provisioning module broke that core goal because it relies on JSOM. Doesn't that make it only run inside the context of SharePoint? So SharePoint add-ins only.

@pbjorklund I started looking at how hard it would be to refactor that code out too. Seems like every packaging experiment I have tried always fails to build in that code. ObjectHandlerBase and schema I am looking directly at you whatever you are. Maybe we could introduce that functionality back in on top of the REST API later? Assuming the JSOM code is covered by the REST API of course. I am still not clear on what that code does.

[mike-morr]

@pbjorklund Here is my understanding of es6 modules.

Code (all in one file named car.js.  the entire file is the module.):  

export default class car { // default
  title = "Clunker";
}

export class carTools {  // no default
  rename() {}
}

export function doStuff() {} //no default

var temp = 123;  // This is internal not accessible via import.

---

Usage:

import clunk123 from 'car'

clunk123.title should equal "Clunker".  clunk123 could be any identifier.  This just takes the default export and assigns it to whatever you want to refer to it as.  Like the Highlander, there can be only one.  Default export.

import {carTools} from 'car'

carTools.rename() should be executable.  This style cherry picks a symbol out of the module (could also be a function or variable you are cherry picking as long as it's exported.

import * as car3 from 'car'

car3.car.title should equal clunk123
car3.carTools.rename() should execute.
car3.doStuff() should execute

This style says give me everything and hang it off car3 (or whatever else you want to refer to it as)

This sytax is also valid using a combination of the above.

import car4, {carTools, doStuff} from 'car' should also work.

car4 gets the default and the other 2 get cherry picked.

[pbjorklund]

@mike-morrison thanks, but I was more talking about why in the browser when we use requirejs to load the referenced file it comes out as pnp.defaults with that { _esModule, defaults } object

[patrick-rodgers]

The real issue is this:

Both CommonJS and AMD generally have the concept of an exports object which contains all exports from a module.

They also support replacing the exports object with a custom single object. Default exports are meant to act as a replacement for this behavior; however, the two are incompatible. TypeScript supports export = to model the traditional CommonJS and AMD workflow.

The export = syntax specifies a single object that is exported from the module. This can be a class, interface, namespace, function, or enum.

When importing a module using export =, TypeScript-specific import let = require("module") must be used to import the module.

That is from the TypeScript docs. In changing the source code to support the es6 include syntax when you add the npm package it breaks the functionality we had to include and use the pnp object directly (which is key to usability in my opinion). I am working on what options we might have here but it looks like we can't support both es6 syntax and export = at the same time.

[mike-morr]

From later on that same page:

Depending on the module target specified during compilation, the compiler will generate appropriate code for Node.js (CommonJS), require.js (AMD), isomorphic (UMD), SystemJS, or ECMAScript 2015 native modules (ES6) module-loading systems. For more information on what the define, require and register calls in the generated code do, consult the documentation for each module loader.

I am pretty sure, we just need to write ES6 modules and let the compiler generate the packages we want to support. That "export =" syntax looks old. I haven't seen anyone using that syntax in a long time. I think the documentation was written to say if the code you are consuming is old and looks like that, you have to import it a different way. I remember having to write syntax like that in the TS earlier days.

I'll see if I can get some clarification from the TS team.

[patrick-rodgers]

You're missing the issue. Yes we can use the compiler to build the different module types. The problem is that we want to support the ability in JavaScript to require/include in a script tag the code and be able to go pnp.sp.* etc. That is what the export = X syntax supports, the replacement of the exports with the object itself. I am working on just replacing the script during package to work how we want in the browser while still supporting the es6 syntax when using the npm module.

[pbjorklund]

I guess the whole new Core() got no traction whatsoever.

Reverted those changes in this PR and moved over to the .replace() solution already in dev. Can open a new issue to discuss the best way to expose the pnp object at runtime there.

I rebased this on split-types before, can extract that into a separate PR if needed. We dont' need 2 that touch the same things.

[pbjorklund]

Ok. Sorted.

Moved util to #86
Moved types to #89
Abandoned the move of provisioning and created an issue instead #87
Moved discussion about exposure of pnp to #88