Get ready to publish Alloy as an NPM repo #632

jonsnyder · 2020-11-03T21:20:47Z

Description

This PR gets the repo ready to be published as an NPM library. Other PRs will automate the build process. The NPM library publishes an ES6 module which exposes the baseCode, core initialization, and the createEventMergeId functions. The built baseCode and standalone javascript files are not published in the NPM library. If you want to use the built versions you should use the version on the CDN.

There are three entry points now:

src/baseCode.js - used to generate the base code snippet
src/standAlone.js - used to generate the standalone code
src/index.js - used to generate the ES6 modules

I was also able to remove the REACTOR pre-processor statements, as the launch extension can use the ES6 module and the createEventMergeId directly.

Related Issue

https://jira.corp.adobe.com/browse/CORE-52500

Motivation and Context

We don't want to have to copy built files as part of the Launch Extension release process. The launch extension can use the NPM library to integrate.
Publishing an NPM library allows customers to integrate with their javascript build system.
If we were to export more modules we could allow customers to do custom builds. (i.e. exclude the personalization module).

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (non-breaking change which does not add functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA or I'm an Adobe employee.
I have made any necessary test changes and all tests pass.
I have run the Sandbox successfully.

jonsnyder · 2020-11-03T21:22:20Z

package.json

+  "module": "dist/index.js",
+  "files": [
+    "dist/index.js"
+  ],


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

jonsnyder · 2020-11-03T21:31:26Z

src/constants/libraryName.js

@@ -10,8 +10,4 @@ OF ANY KIND, either express or implied. See the License for the specific languag
 governing permissions and limitations under the License.
 */

-/* #if _REACTOR
-export default "https://ns.adobe.com/experience/alloy/reactor";


I'm thinking that for the launch extension, we can use the "onBeforeEventSend" function to update the implementation name and version.

That might work. One concern is that we log the configuration options out to the console and if we're always adding a callback as an option (if I understand your proposal), then it will always show up in the configuration logged to the console, which might confuse the customer.

Let's try to resolve this in the process: https://jira.corp.adobe.com/browse/CORE-54527

That bug is unfortunate. Yes we should fix that.

I didn't think about the optics of the onBeforeEventSend callback.

jonsnyder · 2020-11-03T21:32:18Z

src/core/index.js

@@ -38,127 +38,129 @@ import injectProcessWarningsAndErrors from "./edgeNetwork/injectProcessWarningsA
 import validateNetworkResponseIsWellFormed from "./edgeNetwork/validateNetworkResponseIsWellFormed";
 import isRetryableHttpStatusCode from "./network/isRetryableHttpStatusCode";

-// eslint-disable-next-line no-underscore-dangle
-const instanceNames = window.__alloyNS;
+export default () => {


wrapping this in a function allows it to be exported as a module.

Aaronius

This is looking great and will be really nice to have. I added some comments that we'll probably need to work through.

Aaronius · 2020-11-05T21:19:07Z

README.md

+
+### 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)


Period at the end preferably.

Aaronius · 2020-11-05T21:22:43Z

README.md

+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.


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.

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.

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.

Aaronius · 2020-11-05T21:24:01Z

coverageignore.js

+  "**/constants/**",
+  "**/index.js",
+  "src/baseCode.js",
+  "src/standAlone.js"


"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.

Aaronius · 2020-11-05T21:30:04Z

package.json

-    "@adobe/reactor-query-string": "^1.0.0",
-    "css.escape": "^1.5.1",
-    "uuid": "^3.3.2"
+    "@adobe/reactor-query-string": "^1.0.0"


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. 🤔

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/

Aaronius · 2020-11-05T21:42:03Z

test/unit/specs/baseCode.spec.js

+*/
+
+// eslint-disable-next-line no-unused-vars
+import baseCode from "../../../src/baseCode";


It looks like you added baseCode.js to .coverageignore. Do you still want this spec file still?

I thought I got an error when I tried to push, I'll re-check.

Aaronius · 2020-11-06T04:26:36Z

rollup.config.js

+        format: "iife"
+      }
+    ],
+    plugins: minify ? [...plugins, baseCodeTerser] : plugins


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.

Aaronius · 2020-11-06T04:50:39Z

rollup.config.js

+    // module build. The Launch extension does not need them included.
+    external(name) {
+      return /^@adobe\/reactor-/.test(name);
+    },


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.

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.

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.

Aaronius · 2020-11-06T04:52:58Z

rollup.config.js

+    input: "src/baseCode.js",
+    output: [
+      {
+        file: `${destDirectory}baseCode${minifiedExtension}.js`,


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.

Yes, you are right that it is not necessary.

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.

Aaronius · 2020-11-06T05:33:25Z

rollup.config.js

+}
+
+config.push({
+  input: "src/standAlone.js",


"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.

Aaronius · 2020-11-06T05:36:05Z

test/unit/specs/components/EventMerge/createComponent.spec.js

+      }
+    });
+  });
+});


When I go to the event merge ID page in the sandbox, I'm seeing this error that's probably related to this refactor:

alloy.js:2956 Uncaught (in promise) TypeError: [alloy] [EventMerge] An error occurred while executing the createEventMergeId command. Caused by: Cannot read property 'apply' of undefined

jonsnyder added 6 commits October 12, 2020 14:42

CORE-52172 expose ES modules so Launch extension can consume npm repo

5548e15

CORE-52172 add tests and ignores, add more terser options

8cdcc34

CORE-52172 add readme

fdf619f

Merge remote-tracking branch 'origin/master' into npmLibrary

ca0139e

CORE-52172 add test files to pass lint test

f4f37ba

CORE-52172 finish readme, setup files array, fix terser options

6213606

jonsnyder commented Nov 3, 2020

View reviewed changes

jonsnyder requested review from Aaronius, jfkhoury and shammowla November 3, 2020 21:34

Aaronius reviewed Nov 6, 2020

View reviewed changes

Aaronius closed this Nov 10, 2020

Aaronius deleted the branch master November 10, 2020 21:17

jonsnyder mentioned this pull request Nov 13, 2020

Get ready to publish Alloy as an NPM repo #637

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get ready to publish Alloy as an NPM repo #632

Get ready to publish Alloy as an NPM repo #632

jonsnyder commented Nov 3, 2020 •

edited

jonsnyder Nov 3, 2020

jonsnyder Nov 3, 2020

Aaronius Nov 6, 2020

jonsnyder Nov 6, 2020

jonsnyder Nov 3, 2020

Aaronius left a comment

Aaronius Nov 5, 2020

Aaronius Nov 5, 2020

jonsnyder Nov 6, 2020

Aaronius Nov 9, 2020

Aaronius Nov 5, 2020

Aaronius Nov 5, 2020

jonsnyder Nov 6, 2020

Aaronius Nov 5, 2020

jonsnyder Nov 6, 2020

Aaronius Nov 6, 2020

Aaronius Nov 6, 2020

Aaronius Nov 6, 2020

jonsnyder Nov 6, 2020

Aaronius Nov 6, 2020

jonsnyder Nov 6, 2020

jonsnyder Nov 13, 2020

Aaronius Nov 6, 2020

Aaronius Nov 6, 2020

jonsnyder Nov 30, 2020


		### 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)

Get ready to publish Alloy as an NPM repo #632

Get ready to publish Alloy as an NPM repo #632

Conversation

jonsnyder commented Nov 3, 2020 • edited

Description

Related Issue

Motivation and Context

Screenshots (if appropriate):

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaronius left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonsnyder commented Nov 3, 2020 •

edited