Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get ready to publish Alloy as an NPM repo #632

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 49 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,56 @@

# Alloy

Alloy is the code name for the Adobe Experience Platform Web SDK. It allows for streaming data into the platform, syncing identities, personalizing content, and more.
Alloy is the code name for the Adobe Experience Platform Web SDK. It allows for recording events into the Adobe Experience Platform, syncing identities, personalizing content, and more.

For documentation on how to use Alloy, please see the [user documentation](https://adobe.ly/36dGGp6).

For documentation on how to contribute to Alloy, please see the [developer documentation](https://github.com/adobe/alloy/wiki).

## Installation

There are three supported ways to use Alloy.
1. Using Adobe Launch, install the Adobe Experience Platform Web SDK Extension.
2. Use the pre-built, minified version available via a CDN. You could also self-host this version.
3. Use the NPM library which exports an ES6 module.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider putting all this in the user docs? I'd like to consolidate documentation in one place if possible. A lot of this duplicated what's in the user docs and would be kind of pain to maintain separate (for example, I needed to update the version inside the script tag on the user docs during the release. Now we'd have to remember to update the version here as well if we want it to always reflect the latest version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think some of this should go in the user docs. I added more info here because the README.md is uploaded as part of the NPM package, and I wanted a quick guide to using it. Thinking about it more now, I think I should just include the ES6 instructions here, and then link to the user docs for the other way of using it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd still prefer having it all in the user docs, but maybe just split into different pages. Something like...

  • Installing the SDK Using Launch
  • Installing the SDK Using Standalone Library
  • Installing the SDK Using NPM Package

or something like that. I like the idea of having all our customer-oriented docs in one place. I think it would be simpler for us to maintain and simpler for our customers to discover.


### Using the launch extension

For documentation on the AEP Web SDK Launch Extension see the [launch documentation](https://docs.adobe.com/content/help/en/launch/using/extensions-ref/adobe-extension/aep-extension/overview.html)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Period at the end preferably.


### Using the stand alone version

```html
<script>
!function(n,o){o.forEach(function(o){n[o]||((n.__alloyNS=n.__alloyNS||
[]).push(o),n[o]=function(){var u=arguments;return new Promise(
function(i,l){n[o].q.push([i,l,u])})},n[o].q=[])})}
(window,["alloy"]);
</script>
<script src="https://cdn1.adoberesources.net/alloy/2.2.0/alloy.min.js" async></script>
<script>
window.alloy("config", { ... });
window.alloy("sendEvent", { ... });
</script>
```

### Using the NPM library

```bash
npm install @adobe/alloy
```

Using the library:

```javascript
import { baseCode, core } from "@adobe/alloy";
baseCode(["alloy"]); // creates the window.alloy function
core(); // runs

window.alloy("config", { ... });
window.alloy("sendEvent", { ... });
```

The ES6 exports are exposed via the NPM "module" property, so you may need to adjust your rollup system to look for the module property.


8 changes: 7 additions & 1 deletion coverageignore.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,10 @@ governing permissions and limitations under the License.
* Patterns of source files (files within the src directory) that should be
* ignored for test coverage checks and reporting.
*/
module.exports = ["**/.*", "**/constants/**", "**/index.js"];
module.exports = [
"**/.*",
"**/constants/**",
"**/index.js",
"src/baseCode.js",
"src/standAlone.js"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"standalone" is a single word, so "A" should be lowercase. It looks like you got it correct on the file itself, but maybe just forgot to change it here.

];
47 changes: 28 additions & 19 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 17 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
{
"name": "@adobe/alloy",
"version": "2.2.0",
"description": "Client-Side SDK for Unified Data Collection",
"main": "src/core/index.js",
"description": "Adobe Experience Platform Web SDK",
"module": "dist/index.js",
"files": [
"dist/index.js"
],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This array limits the files that are uploaded as part of the NPM library. "package.json", "LICENSE", and "README.md" are included automatically.

"scripts": {
"clean": "rimraf dist",
"lint": "eslint --fix \"*.js\" \"src/**/*.js\" \"test/**/*.js\"",
Expand All @@ -28,12 +31,10 @@
"prebuild": "npm run clean && npm run format && npm run lint",
"build": "rollup -c",
"prebuild:prod": "npm run prebuild",
"build:prod:standalone:unminified": "rollup -c --environment BUILD:prodStandalone",
"build:prod:standalone:minified": "rollup -c --environment MINIFY,BUILD:prodStandalone",
"build:prod:reactor:unminified": "rollup -c --environment BUILD:prodReactor",
"build:prod": "npm-run-all --parallel build:prod:standalone:unminified build:prod:standalone:minified build:prod:reactor:unminified",
"build:prod:unminified": "rollup -c --environment BUILD:prod",
"build:prod:minified": "rollup -c --environment MINIFY,BUILD:prod",
"build:prod": "npm-run-all --parallel build:prod:unminified build:prod:minified",
"build:watch": "rollup -c -w",
"build:basecode": "terser src/baseCode/index.js --mangle --compress unused=false",
"sandbox": "cd sandbox && export REACT_APP_NONCE=321 && npm start",
"sandbox:install": "cd sandbox && npm install",
"sandbox:build": "cd sandbox && npm run build",
Expand Down Expand Up @@ -77,14 +78,20 @@
}
],
"dependencies": {
"css.escape": "^1.5.1",
"uuid": "^3.3.2"
},
"peerDependencies": {
"@adobe/reactor-cookie": "^1.0.0",
"@adobe/reactor-load-script": "^1.1.1",
"@adobe/reactor-object-assign": "^1.0.0",
"@adobe/reactor-query-string": "^1.0.0",
"css.escape": "^1.5.1",
"uuid": "^3.3.2"
"@adobe/reactor-query-string": "^1.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh this is interesting. I can see why you did this for when alloy is installed as a dependency of the extension. It does kind of suck for the customer wanting to use the npm package in their app though because it's unlikely they'll already have these packages installed and they'll have to install them all. I'm not sure how else you would handle it though. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point, I should add some info to the README.md to mention that these need to be installed. NPM v7 adds support for automatically installing peer dependencies. https://github.blog/2020-10-13-presenting-v7-0-0-of-the-npm-cli/

},
"devDependencies": {
"@adobe/reactor-cookie": "^1.0.0",
"@adobe/reactor-load-script": "^1.1.1",
"@adobe/reactor-object-assign": "^1.0.0",
"@adobe/reactor-query-string": "^1.0.0",
"@babel/core": "^7.2.2",
"@babel/plugin-proposal-object-rest-spread": "^7.3.2",
"@babel/plugin-transform-template-literals": "^7.4.4",
Expand Down
98 changes: 46 additions & 52 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,19 @@ governing permissions and limitations under the License.
*/

import path from "path";
import jscc from "rollup-plugin-jscc";
import resolve from "rollup-plugin-node-resolve";
import commonjs from "rollup-plugin-commonjs";
import babel from "rollup-plugin-babel";
import { terser } from "rollup-plugin-terser";
import license from "rollup-plugin-license";

const buildTargets = {
PROD_STANDALONE: "prodStandalone",
PROD_REACTOR: "prodReactor",
PROD: "prod",
DEV: "dev"
};

const destDirectoryByBuildTarget = {
[buildTargets.PROD_STANDALONE]: "dist/standalone/",
[buildTargets.PROD_REACTOR]: "dist/reactor/",
[buildTargets.PROD]: "dist/",
[buildTargets.DEV]: "sandbox/public/"
};

Expand All @@ -35,14 +32,14 @@ const minify = process.env.MINIFY;
const destDirectory = destDirectoryByBuildTarget[buildTarget];

const minifiedExtension = minify ? ".min" : "";
const baseCodeTerser = terser({
mangle: true,
compress: {
unused: false
}
});

const plugins = [
jscc({
values: {
_DEV: buildTarget === buildTargets.DEV,
_REACTOR: buildTarget === buildTargets.PROD_REACTOR
}
}),
resolve({
preferBuiltins: false,
// Support the browser field in dependencies' package.json.
Expand All @@ -53,10 +50,6 @@ const plugins = [
babel()
];

if (minify) {
plugins.push(terser());
}

if (buildTarget !== buildTargets.DEV) {
plugins.push(
license({
Expand All @@ -69,52 +62,53 @@ if (buildTarget !== buildTargets.DEV) {
);
}

const config = {
input: "src/core/index.js",
const config = [];

if (buildTarget === buildTargets.PROD) {
config.push({
input: "src/baseCode.js",
output: [
{
file: `${destDirectory}baseCode${minifiedExtension}.js`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you see this file being used? Maybe it's not necessary? I believe npm package consumers would just use the export from dist/index.js and any other consumer would just copy/paste the simpler version of it from our documentation into their HTML page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right that it is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After having merged in main, and including the changes you made for the tests to use the baseCode, this is necessary for running the tests.

format: "iife"
}
],
plugins: minify ? [...plugins, baseCodeTerser] : plugins
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the license banner is getting stripped out by terser for the minified file. You might need to flip the order of the terser and banner plugins or you might be able to configure the banner or terser in a way that the banner doesn't get stripped during minification.

});
}

config.push({
input: "src/standAlone.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"standalone" is a single word, so "A" should be lowercase. It looks like you got it correct on the file itself, but maybe just forgot to change it here.

output: [
{
file: `${destDirectory}alloy${minifiedExtension}.js`,
// For the Reactor-specific build, we need to use the CommonJS format
// for output instead of IIFE, otherwise Rollup doesn't know whether the
// "external" modules (as defined elsewhere in this configuration) are
// coming from global variables or from another CommonJS bundler, so
// it adds a bunch of cruft to accommodate and logs build warnings.
// Because we have to use the CommonJS format for the Reactor-specific
// build, we can't just use an "intro" that looks like:
// if (IE less than version 11) {
// console.warn("unsupported browser");
// return;
// }
// because you can't "return" from a CommonJS module. Therefore, we have
// to do an if-else for an "intro" and "outro".
// If we do an if-else, we're not compliant with strict mode, because
// babel inserts its function declarations directly inside our if-else.
// Because function declarations are not allowed directly within an
// if-else in strict mode, we need to wrap babel's
// function declarations (and our own code) inside an IIFE.
// So, what we end up with is output using "format: cjs" but with an
// appropriate intro and outro that provides browser checking and an
// IIFE. This works for both Reactor-specific and standalone use cases.
format: "cjs",
// Allow non-IE browsers and IE11
// document.documentMode was added in IE8, and is specific to IE.
// IE7 and lower are not ES5 compatible so will get a parse error loading
// the library.
format: "iife",
intro:
"if (document.documentMode && document.documentMode < 11) {\n" +
" console.warn('The Adobe Experience Cloud Web SDK does not support IE 10 and below.');\n" +
"} else {\n" +
" (function() {",
outro: " })();\n}"
" return;\n" +
"}\n"
}
],
plugins
};
plugins: minify ? [...plugins, terser()] : plugins
});

// If we're building for Reactor, we'll use Reactor's core modules
// (named @adobe/reactor-*) instead of including the packages directly.
if (buildTarget === buildTargets.PROD_REACTOR) {
config.external = name => /^@adobe\/reactor-/.test(name);
if (buildTarget === buildTargets.PROD && !minify) {
config.push({
input: "src/index.js",
output: [
{
file: `${destDirectory}index.js`,
format: "es"
}
],
// The @adobe/reactor-* dependencies are specified as peerDependencies so no need to include them in the
// module build. The Launch extension does not need them included.
external(name) {
return /^@adobe\/reactor-/.test(name);
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the module we're emitting largely being transpiled to ES5 (like const -> let) but is retaining ES6 imports:

import assign from '@adobe/reactor-object-assign';
import cookie from '@adobe/reactor-cookie';
import queryString from '@adobe/reactor-query-string';
import loadScript from '@adobe/reactor-load-script';

and ES6 exports:

export { index as baseCode, index$1 as core, createEventMergeId };

Launch doesn't support es6 yet and we're using es5 imports in the extension's library modules (as we saw here, so we'll probably want to make dist/index.js be an es5 module or we modify the extension to be able to use dist/index.js as it is. Here's a bit more to think about though. When publishing the npm package for general use, my experience has been that a lot of developers want access to the ES6 code because (1) they're targeting only browsers that support ES6 and don't want the weight that comes with ES5-transpiled code and (2) they get better tree-shaking. As it is, we're giving them an ES6 module largely transpiled to ES5. I think it would be best to publish a fully ES6 module (with const, imports, and exports) and fully ES5 module (with var and commonJS). If you were to publish a fully ES5 one, which uses commonJS modules instead of ES6 modules, then the Launch extension should just be able to use the fully ES5 version without much hassle.

FWIW, with Penpal I tried to cut off publishing transpiled ES5 code and only publish ES6 code, hoping that bundlers would have come far enough that if consumers wanted to target older browsers, they could just configure their bundler to transpile Penpal accordingly. Unfortunately, with the current state of the JS ecosystem, this was too much of a burden and people asked me to add back publishing of ES5 code alongside ES6 code, which I did. If you set up package.json like this, then the developer's bundler should be smart enough to use the ES6 one when it can and fall back to the ES5 version if it can't.

I think I kind of rambled there, but hopefully that makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing that we may have discussed but is worth describing at this point is that (1) when we build the Alloy extension, we're not running any sort of bundler on the extension library modules and (2) Launch's bundler that generates the Launch library doesn't support extension library modules requiring npm packages. In other words, our extension library module can't do require("alloy") because Launch doesn't do any npm package resolution except for Launch core modules (e.g., require("@adobe/reactor-object-assign")). Launch may support npm package resolution in the future, but that's a deeper discussion. We could run our own bundler on our extension library code before we deliver our extension package to Launch, which would prevent Launch from having to do any npm resolution.

But, with that said, our code in the extension library modules can require the alloy package by path instead of by name, like this:

require("../../node_modules/@adobe/alloy/dist/index.js")'

Since Launch won't have to do any npm resolution, Launch just treats it as though it were any other relatively referenced file in the extension. This also means we don't have to run our own bundler on our extension library code either. Let me know if that doesn't make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. When I was working on the extension I had to add a rollup step to generate the alloy code, but it would be nice not to have to do that.

plugins
});
}

export default config;
17 changes: 17 additions & 0 deletions src/baseCode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
Copyright 2020 Adobe. All rights reserved.
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/

// This file is used by rollup to create a base code example

import baseCode from "./baseCode/index";

baseCode(["alloy"]);