README details on using external libraries + some reorganization #391

Zlatkovsky · 2017-04-28T19:09:25Z

I added a bunch of detail on how to use external libraries, and also used this as an opportunity for some minor reorganization.

Could one of you take a look and see if it all makes sense and is grammatically sound? @WrathOfZombies, could you also help make sure I'm using the right terminology and explanation of the various types of modules? Thanks!

WrathOfZombies · 2017-04-28T19:52:43Z

Would you want to split this readme into Wiki's instead? It would be easier to consume and only look for what you want thanks to a TOC.

siewmoi

Changes to non-libraries sections, looks good (for now). I can do a more thorough review/step thru post build. wrt the libraries sections, defer to Bhargav.

markpete · 2017-04-28T19:55:47Z

Is it worth talking about nested library dependencies and how they get loaded? My assumption is that this is all automatic like a normal npm package, but I'm curious if there are cases where you need to add the nested dependencies to the Library list.

siewmoi · 2017-04-28T19:59:14Z

Wikis are great but I found wiki not so portable (compared to md files). When I was doing MS Graph samples we used shared wiki to point ppl to small how to this and that. Keeping track of them was a pain too. From: Bhargav Krishna [mailto:[email protected]] Sent: Friday, April 28, 2017 12:53 PM To: OfficeDev/script-lab <[email protected]> Cc: Siew Moi Khor <[email protected]>; Review requested <[email protected]> Subject: Re: [OfficeDev/script-lab] README details on using external libraries + some reorganization (#391) Would you want to split this readme into Wiki's instead? It would be easier to consume and only look for what you want thanks to a TOC. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub<OfficeDev/script-lab#391 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AJ-LPNrcUGTb9lWcnN3f04t1wkez2cPEks5r0kOMgaJpZM4NL3Im>.

markpete · 2017-04-28T19:59:53Z

Also, what version(s) of ES does ScriptLab support? I'm assuming ES5/6 yes and ES7 no. Is it the same as Edge, IE, or Chrome? Chakra?

WrathOfZombies

After reading through it, I feel it would benefit both the way we document stuff and engagement if we went ahead and shaved off some content and added it here:

https://medium.com/office-app-development

and link that in this readme as good to read. Also linking https://basarat.gitbooks.io/typescript/content/docs/getting-started.html will help.

WrathOfZombies · 2017-04-28T19:54:31Z

README.md

+> ![Import](.github/images/unpkg.jpg)
+
+
+#### Moderate: NPM package is an AMD module, but the library is also available via a "dist" build


Wrong title for the given example. An AMD module is one that is loaded from require.js

dont mention "dist" in title as its not a hard and fast rule to have stuff available from dist.

Based on your further docs, i think you mean to add a NOT?

NPM package is an **NOT** AMD module, b

WrathOfZombies · 2017-04-28T19:56:46Z

README.md

+     "https://unpkg.com/[email protected]",
+     3302, 1, ReferenceError: module is not defined {}]
+
+If you are lucky, even though the *NPM* package doesn't contain a web-ready version of the package, the web-compatible version may be available elsewhere.  In the case of this `javascript-qrcode` package, the [NPM entry](https://www.npmjs.com/package/javascript-qrcode) points to a GitHub repo, <https://github.com/siciarek/javascript-qrcode>. And the repo, in turn, has a "dist" folder (convention), along with exampe usage that also shows `<script src="dist/qrcode.min.js"></script>`.


Instead taking a fail first approach would help, as the developer to verify that the library is web compatible. Then try and reference via unpkg. If it still doesnt work, examine the contents of the package, try rawgit. else post an issue on the developers repository to expose an npm package.

WrathOfZombies · 2017-04-28T19:58:57Z

README.md

+
+This just means that you need to have required an additional library, tinycolor.  Again, find its location (with https://unpkg.com being the easiest way of procuring it), and add it to the config paths list (*again, remembering to omit just the `.js` part of the filename). Also include the library name in the array in the `require` statement.  When you patch up both bits, you will end up with the following:
+
+    // At the top of the file:


mark code range with typescript instead of tabs/4 spaces.. and end it with
This will allow you to get coloring.

WrathOfZombies · 2017-04-28T20:04:40Z

README.md

+
+In your libraries tab, add a reference to `require.js` and to its IntelliSense types (note that `require.js` itself should be loaded from a web-compatible CDN version, not its NPM package):
+
+    https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.3/require.min.js


use unpkg!!!

requirejs @types/requirejs

WrathOfZombies · 2017-04-28T20:05:07Z

README.md

+Include this in your Libraries tab, and you're good to go!
+
+
+#### Harder: NPM package is an AMD module (must be loaded with `require.js`), and is *not* available in "dist" form


Technically this is the medium difficulty one. Given that there's a possibility of loading stuff in.

@Zlatkovsky a todo would be adding a require.config tab when we see require.js being a dependency.

I love the idea of making require a native part of ScriptLab. It's pretty commonly used across the web app world these days.

WrathOfZombies · 2017-04-28T20:06:59Z

README.md

+        console.log(gradient.rgb(10));
+    });
+
+If you're lucky, it will work right off the bat.  If you're slightly less lucky, you might still get a useful error that will help you fix the issue (which is why the `require.onError` line described in step #1 is so important!)


Something about this feels weird. Luck has nothing to do with it.
Its code. there's either a true or a false.

WrathOfZombies · 2017-04-28T20:08:56Z

README.md

+    Error: Script error for "tinycolor", needed by: tinygradient
+    http://requirejs.org/docs/errors.html#scripterror {...}
+
+This just means that you need to have required an additional library, tinycolor.  Again, find its location (with https://unpkg.com being the easiest way of procuring it), and add it to the config paths list (*again, remembering to omit just the `.js` part of the filename). Also include the library name in the array in the `require` statement.  When you patch up both bits, you will end up with the following:


I feel we are teaching folks how to use require.js today. In that case how do we plan to support say system.js or X.js when its out tomorrow. May be we should avoid this. Have it as a blog post on your personal blog or medium saying "How do you load AMD libraries in Script Lab". There you can explain all you want.

WrathOfZombies

If you want let's have a chat first about this before we make it public. I feel certain things are out of place.

WrathOfZombies · 2017-04-28T20:10:45Z

@markpete we support upto ES7 and we down compile back to ES5.

WrathOfZombies · 2017-04-28T20:12:21Z

@siewmoi Could you explain what you meant by port? Where are we porting them to? Also why would we port something on this repository elsewhere?

@WrathOfZombies - if we don't foresee a need to port, then it's ok to use wiki.
some background: wrt to our MS Graph samples, we had many repos, one for each platform, etc and to top it they were also getting started samples on dev.office.com. We used the wiki during preview and had to move to dev.office.com repo at GA.

README details on using external libraries + some reorganization

d6c2a7d

Zlatkovsky requested review from WrathOfZombies, jakobpn and siewmoi April 28, 2017 19:09

siewmoi approved these changes Apr 28, 2017

View reviewed changes

WrathOfZombies suggested changes Apr 28, 2017

View reviewed changes

WrathOfZombies reviewed Apr 28, 2017

View reviewed changes

WrathOfZombies suggested changes Apr 28, 2017

View reviewed changes

		> ![Import](.github/images/unpkg.jpg)


		#### Moderate: NPM package is an AMD module, but the library is also available via a "dist" build


		This just means that you need to have required an additional library, tinycolor. Again, find its location (with https://unpkg.com being the easiest way of procuring it), and add it to the config paths list (*again, remembering to omit just the `.js` part of the filename). Also include the library name in the array in the `require` statement. When you patch up both bits, you will end up with the following:

		// At the top of the file:


		In your libraries tab, add a reference to `require.js` and to its IntelliSense types (note that `require.js` itself should be loaded from a web-compatible CDN version, not its NPM package):

		https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.3/require.min.js

		Include this in your Libraries tab, and you're good to go!


		#### Harder: NPM package is an AMD module (must be loaded with `require.js`), and is not available in "dist" form

README details on using external libraries + some reorganization #391

Are you sure you want to change the base?

README details on using external libraries + some reorganization #391

Uh oh!

Conversation

Zlatkovsky commented Apr 28, 2017

Uh oh!

WrathOfZombies commented Apr 28, 2017

Uh oh!

siewmoi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markpete commented Apr 28, 2017

Uh oh!

siewmoi commented Apr 28, 2017 via email

Uh oh!

markpete commented Apr 28, 2017

Uh oh!

WrathOfZombies left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WrathOfZombies left a comment

Choose a reason for hiding this comment

Uh oh!

WrathOfZombies commented Apr 28, 2017

Uh oh!

WrathOfZombies commented Apr 28, 2017 • edited by siewmoi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

siewmoi left a comment •

edited

Loading

WrathOfZombies commented Apr 28, 2017 •

edited by siewmoi

Loading