Documentation Updates (#284)

* Added comments

* Add CONTRIBUTING.md

* Update README.md

Author: jooohhn

CONTRIBUTING.md

@@ -0,0 +1,33 @@
+# Contributing to the Amplitude SDK for JavaScript
+🎉 Thanks for your interest in contributing! 🎉
+
+## Ramping Up
+### Intro
+- There are three ways for SDK to be loaded
+  - Standard NPM package
+  - Snippet in `<script>` tag
+  - RequireJS (may be dropped in future)
+- Workflow is facilitated with both `make` and `yarn` (legacy, may possibly be updated to full `yarn`)
+- Chek out the [Amplitude Instrumentation Explorer]((https://chrome.google.com/webstore/detail/amplitude-instrumentation/acehfjhnmhbmgkedjmjlobpgdicnhkbp)) to help logging events during development
+
+### Architecture
+- `index.js` is the main entrypoint of SDK
+- Stubbed methods are used when client imports via `<script>` snippet
+  - Allows app to not be blocked while real JS SDK is loaded in
+  - Sent events and identifys are tracked with queues
+- Metadata storage (new) vs cookie (old) storage
+  - more of issue with anonymous id, because it uses device id instead of user id
+- UA Parser: Helps identify browsers
+  - might be able to use upstream library and convert results, rather than our fork
+- sameSiteCookie: Sets how public the cookie reading is
+  - `amplitude.getInstance() is necessary even during reuse because of snippet stubbed
+    - only applicable to snippet import
+
+### Setting Up Development
+- Cloning, installing, and building
+```
+git clone [email protected]:amplitude/Amplitude-JavaScript.git
+cd Amplitude-JavaScript
+make # Runs tests and generate builds
+yarn dev # Start development utility. Open localhost:9000 in your browser to access
+```

Makefile

@@ -66,14 +66,14 @@ README.md: $(SNIPPET_OUT) version
 
 $(OUT): node_modules $(SRC) package.json rollup.config.js rollup.min.js rollup.native.js rollup.esm.js rollup.umd.js rollup.umd.min.js
 	@$(JSHINT) --verbose $(SRC)
-	@NODE_ENV=production $(ROLLUP) --config rollup.config.js
-	@NODE_ENV=production $(ROLLUP) --config rollup.esm.js
-	@NODE_ENV=production $(ROLLUP) --config rollup.umd.js
-	@NODE_ENV=production $(ROLLUP) --config rollup.native.js
-	@NODE_ENV=production $(ROLLUP) --config rollup.nocompat.js
+	@NODE_ENV=production $(ROLLUP) --config rollup.config.js # is the snippet build config
+	@NODE_ENV=production $(ROLLUP) --config rollup.esm.js # does not concat dependencies, only has module and dependencies 
+	@NODE_ENV=production $(ROLLUP) --config rollup.umd.js # generates npm version, also usable in require js app
+	@NODE_ENV=production $(ROLLUP) --config rollup.native.js # generates react native build
+	@NODE_ENV=production $(ROLLUP) --config rollup.nocompat.js # may be able to remove
 	@NODE_ENV=production $(ROLLUP) --config rollup.umd.min.js
 	@NODE_ENV=production $(ROLLUP) --config rollup.min.js
-	@NODE_ENV=production $(ROLLUP) --config rollup.nocompat.min.js
+	@NODE_ENV=production $(ROLLUP) --config rollup.nocompat.min.js # may be able to remove
 
 #
 # Target for minified `amplitude-snippet.js` file.

README.md

@@ -13,22 +13,27 @@
 A JavaScript SDK for tracking events and revenue to [Amplitude](https://www.amplitude.com).
 
 ## Installation and Quick Start
-Please visit our :100:[Developer Center](https://developers.amplitude.com/docs/javascript) for instructions on installing and using our the SDK.
+* For using the SDK, please visit our :100:[Developer Center](https://developers.amplitude.com/docs/javascript) for instructions on installing and using our the SDK.
+* For developing the SDK, please visit our [CONTRIBUTING.md](https://github.com/amplitude/Amplitude-JavaScript/blob/master/CONTRIBUTING.md) to get started.
 
 ## Demo Pages
 * A [demo page](https://github.com/amplitude/Amplitude-JavaScript/blob/master/test/browser/amplitudejs.html) showing a simple integration on a web page.
 * A [demo page](https://github.com/amplitude/Amplitude-JavaScript/blob/master/test/browser/amplitudejs-requirejs.html) showing an integration using RequireJS.
 * A [demo page](https://github.com/amplitude/GTM-Web-Demo) demonstrating a potential integration with Google Tag Manager.
 
-## Changelog
-Click [here](https://github.com/amplitude/Amplitude-JavaScript/blob/master/CHANGELOG.md) to view the JavaScript SDK Changelog.
 
 ## React Native
 This library now supports react-native. It has two dependencies on react-native modules you will have to install yourself:
 
 * [react-native-device-info](https://www.npmjs.com/package/react-native-device-info) Tested with version 3.1.4
 * [@react-native-community/async-storage](https://www.npmjs.com/package/@react-native-community/async-storage) Tested with version 1.6.2
 
+## Node.js
+Please visit [Amplitude-Node](https://github.com/amplitude/Amplitude-Node) for our Node SDK.
+
+## Changelog
+Click [here](https://github.com/amplitude/Amplitude-JavaScript/blob/master/CHANGELOG.md) to view the JavaScript SDK Changelog.
+
 ## Upgrading Major Versions and Breaking Changes #
 
 ### 6.0

scripts/version.js

@@ -30,6 +30,7 @@ replaceTextInFile(
 );
 
 // Update integrity hash in snippet
+// Provides extra layer of security. If script changes, it will fail to load
 const sdkText = fs.readFileSync(path.join('.', `amplitude.min.js`), 'utf-8');
 const hash = crypto.createHash('sha384').update(sdkText).digest('base64');
 replaceTextInFile(

src/amplitude-client.js

@@ -1,14 +1,15 @@
+// Core of SDK code
 import Constants from './constants';
 import cookieStorage from './cookiestorage';
 import MetadataStorage from '../src/metadata-storage';
-import getUtmData from './utm';
+import getUtmData from './utm'; // Urchin Tracking Module 
 import Identify from './identify';
 import localStorage from './localstorage';  // jshint ignore:line
 import md5 from 'blueimp-md5';
 import Request from './xhr';
 import Revenue from './revenue';
 import type from './type';
-import UAParser from '@amplitude/ua-parser-js';
+import UAParser from '@amplitude/ua-parser-js'; // Identifying device and browser info (maybe move to backend?)
 import utils from './utils';
 import UUID from './uuid';
 import base64Id from './base64Id';
@@ -51,6 +52,7 @@ var AmplitudeClient = function AmplitudeClient(instanceName) {
   this._identifyId = 0;
   this._lastEventTime = null;
   this._newSession = false;
+  // sequence used for by frontend for prioritizing event send retries
   this._sequenceNumber = 0;
   this._sessionId = null;
   this._isInitialized = false;
@@ -1567,6 +1569,7 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() {
 
 /**
  * Merge unsent events and identifys together in sequential order based on their sequence number, for uploading.
+ * Identifys given higher priority than Events. Also earlier sequence given priority
  * @private
  */
 AmplitudeClient.prototype._mergeEventsAndIdentifys = function _mergeEventsAndIdentifys(numEvents) {

src/amplitude-snippet.js

@@ -1,3 +1,9 @@
+/**
+* Imported in client browser via <script> tag
+* Async capabilities: Interally creates stubbed window.amplitude object until real SDK loaded
+* Stubbed functions keep track of funciton calls and their arguments
+* These are sent once real SDK loaded through another <script> tag
+*/
 (function(window, document) {
   var amplitude = window.amplitude || {'_q':[],'_iq':{}};
   var as = document.createElement('script');

src/amplitude.js

@@ -8,7 +8,7 @@ import { version } from '../package.json';
 import DEFAULT_OPTIONS from './options';
 
 /**
- * Amplitude SDK API - instance manager.
+ * Legacy API of Amplitude SDK - instance manager. Wraps around the current amplitude-client.js which provides more features
  * Function calls directly on amplitude have been deprecated. Please call methods on the default shared instance: amplitude.getInstance() instead.
  * See [Readme]{@link https://github.com/amplitude/Amplitude-Javascript#300-update-and-logging-events-to-multiple-amplitude-apps} for more information about this change.
  * @constructor Amplitude

src/index.js

@@ -1,15 +1,22 @@
 /* jshint expr:true */
+// Entry point
 import Amplitude from  './amplitude';
 
 const old = window.amplitude || {};
 const newInstance = new Amplitude();
 newInstance._q = old._q || [];
+
+/**
+* Instantiates Amplitude object and runs all queued function logged by stubbed methods provided by snippets
+* Event queue allows async loading of SDK to not blocking client's app
+*/
 for (let instance in old._iq) { // migrate each instance's queue
   if (old._iq.hasOwnProperty(instance)) {
     newInstance.getInstance(instance)._q = old._iq[instance]._q || [];
   }
 }
 
+// If SDK is enabled as snippet, process the events queued by stubbed function
 if (BUILD_COMPAT_SNIPPET) {
   newInstance.runQueuedFunctions();
 }

src/metadata-storage.js

@@ -9,7 +9,13 @@ import getLocation from './get-location';
 import localStorage from './localstorage'; // jshint ignore:line
 import topDomain from './top-domain';
 
-class MetadataStorage {
+/**
+* MetadataStorage involves SDK data persistance
+* storage priority: cookies -> localStorage -> in memory 
+* if in localStorage, unable track users between subdomains
+* if in memory, then memory can't be shared between different tabs
+*/
+ class MetadataStorage {
   constructor({storageKey, disableCookies, domain, secure, sameSite, expirationDays}) {
     this.storageKey = storageKey;
     this.disableCookieStorage = !baseCookie.areCookiesEnabled() || disableCookies;
@@ -35,18 +41,21 @@ class MetadataStorage {
     return `${this.storageKey}${suffix ? `_${suffix}` : ''}`;
   }
 
+  /*
+  * Data is saved as delimited values rather than JSO to minimize cookie space
+  * Should not change order of the items
+  */
   save({ deviceId, userId, optOut, sessionId, lastEventTime, eventId, identifyId, sequenceNumber }) {
-    // do not change the order of these items
     const value = [
       deviceId,
-      Base64.encode(userId || ''),
+      Base64.encode(userId || ''), // used to convert not unicode to alphanumeric since cookies only use alphanumeric
       optOut ? '1' : '',
-      sessionId ? sessionId.toString(32) : '0',
-      lastEventTime ? lastEventTime.toString(32) : '0',
+      sessionId ? sessionId.toString(32) : '0', // generated when instantiated, timestamp (but re-uses session id in cookie if not expired) @TODO clients may want custom session id
+      lastEventTime ? lastEventTime.toString(32) : '0', // last time an event was set
       eventId ? eventId.toString(32) : '0',
       identifyId ? identifyId.toString(32) : '0',
       sequenceNumber ? sequenceNumber.toString(32) : '0'
-    ].join('.');
+    ].join('.'); 
 
     if (this.disableCookieStorage) {
       localStorage.setItem(this.storageKey, value);

src/options.js

@@ -16,7 +16,7 @@ export default {
   batchEvents: false,
   cookieExpiration: 365 * 10,
   cookieName: 'amplitude_id', // this is a deprecated option
-  sameSiteCookie: 'Lax',
+  sameSiteCookie: 'Lax', // cookie privacy policy
   cookieForceUpgrade: false,
   deferInitialization: false,
   disableCookies: false,