Skip to content

Commit

Permalink
Don't use node globals in browser context (#10487)
Browse files Browse the repository at this point in the history
  • Loading branch information
swissspidy authored Feb 8, 2022
1 parent 640d1f4 commit 8359db3
Show file tree
Hide file tree
Showing 41 changed files with 112 additions and 120 deletions.
43 changes: 29 additions & 14 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -219,15 +219,16 @@
},
"env": {
"browser": true,
"es6": true
"es2020": true
},
"globals": {
"global": true,
"window": true,
"document": true,
"wp": "readonly",
"process": "readonly",
"__webpack_public_path__": true
"__webpack_public_path__": true,
"WEB_STORIES_CI": "readonly",
"WEB_STORIES_DISABLE_ERROR_BOUNDARIES": "readonly",
"WEB_STORIES_DISABLE_OPTIMIZED_RENDERING": "readonly",
"WEB_STORIES_DISABLE_PREVENT": "readonly",
"WEB_STORIES_DISABLE_QUICK_TIPS": "readonly",
"WEB_STORIES_ENV": "readonly"
},
"settings": {
"import/resolver": {
Expand Down Expand Up @@ -292,6 +293,9 @@
"testing-library",
"jest-dom"
],
"env": {
"node": true
},
"rules": {
"jest/no-hooks": "off",
"jest/prefer-expect-assertions": "off",
Expand Down Expand Up @@ -375,6 +379,9 @@
"extends": [
"plugin:jest/all"
],
"env": {
"node": true
},
"rules": {
"jest/no-hooks": "off",
"jest/prefer-expect-assertions": "off",
Expand Down Expand Up @@ -430,17 +437,25 @@
{
"files": [
"packages/commander/**/*.js",
"packages/fonts/scripts/cli.js",
"packages/migration/src/cli.js",
"packages/templates/scripts/cli.js",
"packages/text-sets/scripts/cli.js"
"packages/fonts/scripts/**/*.js",
"packages/migration/scripts/**/*.js",
"packages/templates/scripts/**/*.js",
"packages/text-sets/scripts/**/*.js"
],
"rules": {
"import/no-useless-path-segments": ["error", {
"noUselessIndex": false
}]
}
},
{
"files": [
"packages/jest-parallel-sequencer/**/*.js"
],
"env": {
"node": true
}
},
{
"files": [
"__mocks__/**/*.js",
Expand All @@ -457,13 +472,13 @@
"packages/e2e-tests/src/config/*.js",
"packages/dashboard/src/karma-tests.cjs",
"packages/story-editor/src/karma-tests.cjs",
"packages/migration/src/**/*.js",
"packages/eslint-import-resolver/**/*.cjs",
"packages/jest-resolver/**/*.cjs",
"packages/fonts/**/*.js",
"packages/commander/**/*.js",
"packages/templates/scripts/cli.js",
"packages/text-sets/scripts/cli.js"
"packages/migration/scripts/**/*.js",
"packages/templates/scripts/**/*.js",
"packages/text-sets/scripts/**/*.js"
],
"extends": [
"plugin:node/recommended",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function PreviewButton() {
// Start a about:blank popup with waiting message until we complete
// the saving operation. That way we will not bust the popup timeout.
try {
const popup = global.open('about:blank', 'story-preview');
const popup = window.open('about:blank', 'story-preview');

if (popup) {
popup.document.write('<!DOCTYPE html><html><head>');
Expand Down
10 changes: 6 additions & 4 deletions .stylelintignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@
.storybook
.wordpress-org
__mocks__
assets/css
__static__
assets
bin
blocks
build
docs
micro-site
includes
node_modules
patches
public
sitemap-generator
tests
third-party
vendor
web-stories-scraper
/*.js
/*.cjs
2 changes: 1 addition & 1 deletion packages/activation-notice/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { StrictMode, render } from '@wordpress/element';
*/
import App from './app';

__webpack_public_path__ = global.webStoriesActivationSettings.publicPath;
__webpack_public_path__ = window.webStoriesActivationSettings.publicPath;

/**
* Initializes the Web Stories dashboard screen.
Expand Down
6 changes: 3 additions & 3 deletions packages/animation/src/utils/test/createKeyframeEffect.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('createKeyframeEffect', () => {
};

const mockKeyframeEffect = jest.fn();
global.KeyframeEffect = mockKeyframeEffect;
window.KeyframeEffect = mockKeyframeEffect;

createKeyframeEffect('target', 'keyframes', timings);
expect(mockKeyframeEffect).toHaveBeenCalledWith('target', 'keyframes', {
Expand All @@ -41,7 +41,7 @@ describe('createKeyframeEffect', () => {
};

const mockKeyframeEffect = jest.fn();
global.KeyframeEffect = mockKeyframeEffect;
window.KeyframeEffect = mockKeyframeEffect;

createKeyframeEffect('target', 'keyframes', timings);
expect(mockKeyframeEffect).toHaveBeenCalledWith(
Expand All @@ -61,7 +61,7 @@ describe('createKeyframeEffect', () => {
};

const mockKeyframeEffect = jest.fn();
global.KeyframeEffect = mockKeyframeEffect;
window.KeyframeEffect = mockKeyframeEffect;

createKeyframeEffect('target', 'keyframes', timings);
expect(mockKeyframeEffect).toHaveBeenCalledWith(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ function StoriesView({
const handleCopyStoryLink = useCallback(
(story) => {
setContextMenuId(-1);
global.navigator.clipboard.writeText(story.link);
window.navigator.clipboard.writeText(story.link);

showSnackbar({
message: sprintf(
Expand Down
2 changes: 1 addition & 1 deletion packages/dashboard/src/karma/fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { AppFrame } from '../components';
import InterfaceSkeleton from '../components/interfaceSkeleton';
import ApiProviderFixture from './apiProviderFixture';

if ('true' === process.env.CI) {
if ('true' === WEB_STORIES_CI) {
configure({
getElementError: (message) => {
const error = new Error(message);
Expand Down
28 changes: 0 additions & 28 deletions packages/dashboard/src/utils/throttleToAnimationFrame.js

This file was deleted.

2 changes: 1 addition & 1 deletion packages/design-system/src/components/keyboard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ function crossesDialogBoundary(target, keyTarget) {
* @return {boolean} True if platform is a Mac.
*/
export function isPlatformMacOS() {
const { platform } = global.navigator;
const { platform } = window.navigator;
return platform.includes('Mac') || ['iPad', 'iPhone'].includes(platform);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/karma-puppeteer-client/src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@
return function () {
var args = Array.prototype.slice.call(arguments, 0);
var name = '__karma_puppeteer_' + methodName;
if (!global[name]) {
if (!window[name]) {
throw new Error('unknown method: ' + name);
}
return global[name].apply(null, args);
return window[name].apply(null, args);
};
}

Expand Down
2 changes: 0 additions & 2 deletions packages/media/src/test/isAnimatedGif.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
* limitations under the License.
*/

/* global __dirname */

/**
* External dependencies
*/
Expand Down
1 change: 0 additions & 1 deletion packages/migration/scripts/cli.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#!/usr/bin/env node
/*
* Copyright 2020 Google LLC
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ function addInlineLetterSpacing(content, letterSpacing) {

return wrapWithSpan(content, `letter-spacing: ${letterSpacing / 100}em`);
}
/* eslint-disable security/detect-non-literal-regexp */

function stripTag(html, tag) {
// This is a very naive strip. Can only remove non-self-closing tags with attributes, which is sufficient here
Expand All @@ -140,7 +139,6 @@ function replaceTagWithSpan(html, tag, style) {
.replace(new RegExp(`<${tag}>`, 'gi'), `<span style="${style}">`)
.replace(new RegExp(`</${tag}>`, 'gi'), '</span>');
}
/* eslint-enable security/detect-non-literal-regexp */

function wrapWithSpan(html, style) {
return `<span style="${style}">${html}</span>`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,12 @@ function updateElement(element) {
if (element.resource.sizes) {
for (const size of Object.keys(element.resource.sizes)) {
// Disable reason: not acting on untrusted user input.
/* eslint-disable security/detect-object-injection */
const data = element.resource.sizes[size];
element.resource.sizes[size] = {
...data,
width: Number(data.width),
height: Number(data.height),
};
/* eslint-enable security/detect-object-injection */
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ function snakeToCamelCaseObjectKeys(obj) {
function updateElement(element) {
if (element?.resource?.sizes) {
for (const [key, value] of Object.entries(element.resource.sizes)) {
// eslint-disable-next-line security/detect-object-injection
element.resource.sizes[key] = snakeToCamelCaseObjectKeys(value);
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/react/src/useWhyDidYouUpdate.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ function useWhyDidYouUpdate(name, props) {
});
}

const isDevelopment = process.env.NODE_ENV === 'development';
const isDevelopment =
typeof WEB_STORIES_ENV !== 'undefined' && WEB_STORIES_ENV === 'development';

export default isDevelopment ? useWhyDidYouUpdate : () => {};
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ function StoryEmbedEdit({
}, [poster]);

useEffect(() => {
if (ref.current && global.AmpStoryPlayer) {
const player = new global.AmpStoryPlayer(global, ref.current);
if (ref.current && window.AmpStoryPlayer) {
const player = new window.AmpStoryPlayer(window, ref.current);
player.load();
}
}, [showLoadingIndicator, showPlaceholder, isResizable]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function deriveUnreadTipsCount(previous, next) {
}

export function deriveAutoOpen(previous, next) {
if (process.env.DISABLE_QUICK_TIPS === 'true') {
if (WEB_STORIES_DISABLE_QUICK_TIPS === 'true') {
return {};
}
if (isInitialHydrate(previous, next)) {
Expand All @@ -107,7 +107,7 @@ export function deriveAutoOpen(previous, next) {
// If there are any unread tips, we respect users last open setting.
// If all tips are read, we want the popup closed regardless of user setting.
export function deriveInitialOpen(persisted) {
if (process.env.DISABLE_QUICK_TIPS === 'true') {
if (WEB_STORIES_DISABLE_QUICK_TIPS === 'true') {
return {};
}
const hasUnreadTips = Boolean(persisted?.unreadTipsCount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const REGISTER_USAGE_URL =
'?payload=02647749feef0d5536c92df1d9cfa38e';

function mockFetch(response, { requestPath, requestMethod }) {
jest.spyOn(global, 'fetch').mockImplementation((url, { method }) => {
jest.spyOn(window, 'fetch').mockImplementation((url, { method }) => {
const path = new URL(url).pathname;
if (path !== requestPath) {
throw new Error(
Expand All @@ -92,8 +92,8 @@ function mockFetch(response, { requestPath, requestMethod }) {

describe('ApiFetcher', () => {
afterEach(() => {
if (global.fetch['mockClear']) {
global.fetch.mockClear();
if (window.fetch['mockClear']) {
window.fetch.mockClear();
}
});

Expand Down Expand Up @@ -130,7 +130,7 @@ describe('ApiFetcher', () => {
filter,
});
expect(result.media[0].name).toBe('photo 29044');
expect(global.fetch).toHaveBeenCalledTimes(1);
expect(window.fetch).toHaveBeenCalledTimes(1);
const fetchArg = fetch.mock.calls[0][0];
const queryString = fetchArg.substring(fetchArg.indexOf('?') + 1);
const queryParams = queryString.split('&');
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('ApiFetcher', () => {
const escapedFilter = 'cat+and++many+dogs';

await apiFetcher.listMedia({ filter });
const fetchArg = global.fetch.mock.calls[0][0];
const fetchArg = window.fetch.mock.calls[0][0];
const queryString = fetchArg.substring(fetchArg.indexOf('?') + 1);
const queryParams = queryString.split('&');
expect(queryParams).toStrictEqual(
Expand All @@ -208,7 +208,7 @@ describe('ApiFetcher', () => {
const escapedFilter = 'Tom+%26+Jerry';

await apiFetcher.listMedia({ filter });
const fetchArg = global.fetch.mock.calls[0][0];
const fetchArg = window.fetch.mock.calls[0][0];
const queryString = fetchArg.substring(fetchArg.indexOf('?') + 1);
const queryParams = queryString.split('&');
expect(queryParams).toStrictEqual(
Expand Down Expand Up @@ -255,7 +255,7 @@ describe('ApiFetcher', () => {
});

expect(result.categories[0].label).toBe('Covid-19');
expect(global.fetch).toHaveBeenCalledTimes(1);
expect(window.fetch).toHaveBeenCalledTimes(1);
const fetchArg = fetch.mock.calls[0][0];
const queryString = fetchArg.substring(fetchArg.indexOf('?') + 1);
const queryParams = queryString.split('&');
Expand Down
3 changes: 2 additions & 1 deletion packages/story-editor/src/app/media/utils/useFFmpeg.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ import {
} from '../../../constants';
import getPosterName from './getPosterName';

const isDevelopment = process.env.NODE_ENV === 'development';
const isDevelopment =
typeof WEB_STORIES_ENV !== 'undefined' && WEB_STORIES_ENV === 'development';

/**
* Checks whether the file size is too large for transcoding.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import { getStoryAmpValidationErrors } from '../storyAmpValidationErrors';

describe('getStoryAmpValidationErrors', () => {
const fetchSpy = jest.spyOn(global, 'fetch');
const windowSpy = jest.spyOn(global, 'window', 'get');
const fetchSpy = jest.spyOn(window, 'fetch');
const windowSpy = jest.spyOn(window, 'window', 'get');

beforeAll(() => {
fetchSpy.mockResolvedValue({
Expand Down
Loading

0 comments on commit 8359db3

Please sign in to comment.