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

Script Loader #1

Merged
merged 7 commits into from
Mar 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/semantic.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: Semantic Release

Choose a reason for hiding this comment

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

Do we expect the semver version of our npm module to relate in any way to the WebSDK versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I expect this module to respect the semver of websdk but not move in lock step. for example, if websdk releases v42 then this module would create a major release changing the version constant in the loader. if a breaking change is needed here, it can release a major bump independently.


on:
pull_request_target:
types:
- opened
- edited
- synchronize

jobs:
check_pr_title:
runs-on: ubuntu-latest

steps:
- name: Validate Pull Request title
uses: amannn/[email protected]
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
2 changes: 2 additions & 0 deletions .npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
.github
.husky
9 changes: 8 additions & 1 deletion jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,14 @@ export default {
// ],

// An object that configures minimum threshold enforcement for coverage results
// coverageThreshold: undefined,
coverageThreshold: {
global: {
statements: 75,
branches: 70,
functions: 75,
lines: 75,
},
},

// A path to a custom dependency extractor
// dependencyExtractor: undefined,
Expand Down
25 changes: 25 additions & 0 deletions package-lock.json

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

22 changes: 20 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"babel-jest": "^26.6.3",
"conventional-changelog-conventionalcommits": "^4.5.0",
"eslint": "^7.19.0",
"eslint-plugin-jest": "^24.1.5",
"eslint-plugin-prettier": "^3.3.1",
"eslint-plugin-square": "^17.0.0",
"husky": "^5.0.9",
Expand All @@ -73,9 +74,26 @@
"env": {
"browser": true
},
"plugins": [
"square",
"jest"
],
"extends": [
"plugin:square/typescript"
]
"plugin:square/typescript",
"plugin:jest/recommended"
],
"rules": {
"spaced-comment": [
"error",
"always",
{
"markers": [
"/"
]
}
],
"@typescript-eslint/triple-slash-reference": "off"
}
},
"lint-staged": {
"*.ts": "eslint --cache --fix",
Expand Down
8 changes: 2 additions & 6 deletions src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ describe('Index', () => {
expect(1 + 2).toEqual(3);
});

describe('Payments', () => {
it('loads', () => {
expect(() => {
Index.payments();
}).not.toThrow();
});
it('exports payments', () => {
expect(Index).toHaveProperty('payments');
});
});
4 changes: 1 addition & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
export function payments(): void {
console.log('🔜');
}
export * from './payments';
19 changes: 19 additions & 0 deletions src/load.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as Load from './load';

describe('Load', () => {
describe('loadSquare', () => {
it('exports loadSquare', () => {
expect(Load).toHaveProperty('loadSquare');
});

it('memoizes loadPromise', () => {
const src = 'https://websdk.squarecdn.com/v0/square.js';
const p1 = Load.loadSquare(src);
const p2 = Load.loadSquare(src);

expect(p1).toStrictEqual(p2);
});

// hard to unit test because of jsdom behaviors. better to trust integration tests
});
});
65 changes: 65 additions & 0 deletions src/load.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/// <reference path='../types/index.d.ts' />

function findScript(src: string): HTMLScriptElement | null {
return document.querySelector<HTMLScriptElement>(`script[src="${src}"]`);
}

function injectScript(src: string): HTMLScriptElement {
const headOrBody = document.head || document.body;

if (!headOrBody) {
throw new Error('Square.js requires a <body> or <head> element.');
}

const script = document.createElement('script');
script.src = src;

headOrBody.appendChild(script);

return script;
}

let loadPromise: Promise<Square | null> | null = null;

export function loadSquare(src: string): Promise<Square | null> {
if (loadPromise !== null) {
return loadPromise;
}

loadPromise = new Promise((resolve, reject) => {
if (typeof window === 'undefined') {
// Resolve to null when imported server side. This makes the module safe to import in an isomorphic code base.
resolve(null);
return;
}

if (window.Square) {
resolve(window.Square);

Choose a reason for hiding this comment

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

do we want to verify this namespace is valid/ours first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could be more explicit, but I'm not exactly sure how to right away (early days). currently, it'll be implicitly validated when payments is called on it.

return;
}

try {
let script = findScript(src);

Choose a reason for hiding this comment

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

I'm trying to understand the reason why a script might already be present given we're caching the promise value. Is this an edge case, or something we expect to happen often?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

definite edge case in an attempt not to double-load by accident


if (!script) {
script = injectScript(src);
}

script.addEventListener('load', () => {
if (window.Square) {
resolve(window.Square);
} else {
reject(new Error('Square.js failed to load properly.'));
}
});

script.addEventListener('error', () => {
reject(new Error('Error occurred while loading Square.js'));
});
} catch (err) {
reject(err);
}
});

return loadPromise;
}
56 changes: 56 additions & 0 deletions src/payments.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import * as Payments from './payments';
import { loadSquare } from './load';

jest.mock('./load');

const mockLoadSquare = loadSquare as jest.MockedFunction<typeof loadSquare>;

describe('Payments', () => {
beforeEach(() => {
mockLoadSquare.mockClear();
});

describe('payments', () => {
it('exports payments', () => {
expect(Payments).toHaveProperty('payments');
});

it('throws if application id is invalid and has no override', async () => {
await expect(Payments.payments('junk-app-id')).rejects.toThrow(
"The Payment 'applicationId' option is not in the correct format."
);
expect(mockLoadSquare).not.toBeCalled();
});

it('allows overriding script src', async () => {
mockLoadSquare.mockResolvedValueOnce(null);

const testSrc = 'https://square.test/unit.js';

await Payments.payments('sq0idp-...', '', { scriptSrc: testSrc });

expect(mockLoadSquare).toHaveBeenCalledWith(testSrc);
});

it('can resolve null', async () => {
mockLoadSquare.mockResolvedValueOnce(null);

const maybePayments = await Payments.payments('sq0idp-...');
expect(maybePayments).toBeNull();
});

it('resolves window.Square', async () => {
const expected = true;
const SQish = {
payments() {
return expected;
},
};
mockLoadSquare.mockResolvedValue((SQish as unknown) as Square);

const actual = await Payments.payments('sandbox-sq0idb-...');

expect(actual).toBe(expected);
});
});
});
53 changes: 53 additions & 0 deletions src/payments.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { loadSquare } from './load';

const Version = 'v0';

class InvalidApplicationIdError extends Error {
constructor(
message = "The Payment 'applicationId' option is not in the correct format."
) {
super(message);
this.name = 'InvalidApplicationIdError';
Object.setPrototypeOf(this, InvalidApplicationIdError.prototype);
}
}

function getSrcForApplicationId(applicationId: string): string {
let src = '';

if (applicationId.startsWith('sq0idp-')) {
src = 'https://websdk.squarecdn.com/';
}

if (applicationId.startsWith('sandbox-sq0idb-')) {
src = 'https://sandbox.websdk.squarecdn.com/';
}

if (src.length === 0) {
Copy link

@prettymuchbryce prettymuchbryce Feb 25, 2021

Choose a reason for hiding this comment

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

Worth a reminder here that this assumption is somewhat flawed. There are application IDs in the wild that don't use the prefixes.

throw new InvalidApplicationIdError();
}
src += `${Version}/square.js`;

return src;
}

export async function payments(
applicationId: string,
locationId?: string,
overrides?: {
scriptSrc?: string;
}
): Promise<Payments | null> {
const src =
overrides?.scriptSrc !== undefined
? overrides.scriptSrc
: getSrcForApplicationId(applicationId);

const maybeSquare = await loadSquare(src);

Choose a reason for hiding this comment

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

handle rejections?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the next step is to create some examples to better understand how this will be used which should hopefully inform what's useful to handle versus let bubble up. currently, I believe only bad script loads would cause rejections.


if (maybeSquare === null) {
return null;
}

return maybeSquare.payments(applicationId, locationId);
}
11 changes: 11 additions & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
interface Payments {
verifyBuyer: () => void;

Choose a reason for hiding this comment

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

// TODO: Add all of our typings? :)

}

interface Square {
payments: (applicationId: string, locationId?: string) => Promise<Payments>;
}

interface Window {
Square?: Square;
}