Skip to content

Commit

Permalink
feat(Node): use ESM for nearly all modules
Browse files Browse the repository at this point in the history
Unify our files to be mostly ECMAScript modules, rather than mixed,
since it is supported in the current LTS version of Node
https://nodejs.org/docs/latest-v18.x/api/esm.html

CommonJS modules get the explicit `.cjs` / `.cts` extension

Jest tests uses the unstable ESM mocking API, which requires
mocked modules to be imported after the mock using dynamic imports.
https://jestjs.io/docs/ecmascript-modules#module-mocking-in-esm

CDK now uses the ESM version of `ts-node` which loads CommonJS
for `.cts` files, reducing the difference between tsconfig.json files,
which requires `allowImportingTsExtensions` to be true.
https://www.typescriptlang.org/tsconfig#allowImportingTsExtensions
  • Loading branch information
mxdvl committed Sep 22, 2023
1 parent b0ad333 commit 84a4050
Show file tree
Hide file tree
Showing 95 changed files with 690 additions and 362 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pr-deployment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
- name: Boot server and run ngrok
run: |
npm install -g ngrok
NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true node dotcom-rendering/dist/server.js &
NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true node dotcom-rendering/dist/server.cjs &
timeout 5h ngrok http 9000 -log=stdout | \
grep --line-buffered -o 'https://.*' | \
xargs -L1 -I{} -t \
Expand Down
3 changes: 2 additions & 1 deletion dotcom-rendering/.eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
node_modules
.eslintrc.js
.eslintrc.cjs
cypress.config.js
storybook-static/
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,15 @@ module.exports = {
},
overrides: [
{
files: ['**/**.js'],
files: ['**/**.cjs'],
rules: {
'global-require': 'off',
'@typescript-eslint/no-var-requires': 'off',
},
},
{
files: ['**/**.js', '**/**.cjs'],
rules: {
'@typescript-eslint/no-unsafe-member-access': 'off',
'@typescript-eslint/no-misused-promises': 'off',
},
Expand All @@ -195,7 +200,7 @@ module.exports = {
},
},
{
files: ['**/**.d.ts'],
files: ['**/**.d.ts', '**/**.test.ts?(x)'],
rules: {
'@typescript-eslint/consistent-type-imports': [
'error',
Expand Down
1 change: 1 addition & 0 deletions dotcom-rendering/.prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ node_modules/
!/index.d.ts
!/*.js
!package.json
!/*.cjs

# Ignore specific files
/**/*/curl-with-js-and-domReady.js
File renamed without changes.
2 changes: 1 addition & 1 deletion dotcom-rendering/Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ EXPOSE 9000
ENV DISABLE_LOGGING_AND_METRICS=true
ENV NODE_ENV=production

ENTRYPOINT ["node", "rendering/dist/server.js"]
ENTRYPOINT ["node", "rendering/dist/server.cjs"]
3 changes: 2 additions & 1 deletion dotcom-rendering/__mocks__/svgMock.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const SVG = () => null;

module.exports = SVG;
// eslint-disable-next-line import/no-default-export -- it’s what Jest wants
export default SVG;
3 changes: 2 additions & 1 deletion dotcom-rendering/babel.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
module.exports = {
// eslint-disable-next-line import/no-default-export -- it’s what Babel wants
export default {
plugins: [
// Be careful when adding plugins here!
// They can dramatically alter the build size
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/cdk.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"app": "npx ts-node --project=tsconfig.cdk.json cdk/bin/cdk.ts",
"app": "npx ts-node --esm --project=tsconfig.cdk.json cdk/bin/cdk.cts",
"context": {
"aws-cdk:enableDiffNoFail": "true",
"@aws-cdk/core:stackRelativeExports": "true"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import 'source-map-support/register';
import { App } from 'aws-cdk-lib';
import { DotcomRendering } from '../lib/dotcom-rendering';
import { DotcomRendering } from '../lib/dotcom-rendering.cts';

const app = new App();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, expect, it } from '@jest/globals';
import { App } from 'aws-cdk-lib';
import { Template } from 'aws-cdk-lib/assertions';
import { DotcomRendering } from './dotcom-rendering';
import { DotcomRendering } from './dotcom-rendering.cts';

/**
* These tests make sure that the cloudformation template being generated by CDK has not deviated
Expand Down
30 changes: 11 additions & 19 deletions dotcom-rendering/cypress.config.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import path from 'node:path';
import webpackPreprocessor from '@cypress/webpack-preprocessor';
import { defineConfig } from 'cypress';
import { babelExclude } from './scripts/webpack/webpack.config.client.js';
import { swcLoader } from './scripts/webpack/webpack.config.server.js';

// https://docs.cypress.io/guides/references/configuration

module.exports = defineConfig({
// eslint-disable-next-line import/no-default-export -- it’s what Cypress wants
export default defineConfig({
viewportWidth: 1500,
viewportHeight: 860,
video: false,
Expand Down Expand Up @@ -34,25 +35,16 @@ module.exports = defineConfig({

const webpackConfig = webpackPreprocessor.defaultOptions;
webpackConfig.webpackOptions.resolve = {
extensions: ['.ts', '.js'],
extensions: ['.js', '.ts', '.tsx', '.cts', '.ctsx'],
};
const rules = webpackConfig.webpackOptions.module.rules;
rules[0].exclude = babelExclude;

// Adding this here so that we can import the fixture in the sign-in-gate.cy.js file
rules.push({
test: path.resolve(
__dirname,
`./fixtures/generated/articles/Standard.ts`,
),
exclude: ['/node_modules/'],
loader: 'ts-loader',
options: {
compilerOptions: {
noEmit: false,
},
webpackConfig.webpackOptions.module.rules = [
{
test: /\.c?tsx?$/,
exclude: babelExclude,
use: swcLoader,
},
});
];

on('file:preprocessor', webpackPreprocessor(webpackConfig));
return config;
},
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion dotcom-rendering/cypress/e2e/parallel-1/article.e2e.cy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { disableCMP } from '../../lib/disableCMP.js';
import { setUrlFragment } from '../../lib/setUrlFragment.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { mockApi } from '../../lib/mocks';
import { mockApi } from '../../lib/mocks.js';

describe('E2E Page rendering', function () {
beforeEach(function () {
Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/cypress/e2e/parallel-1/sign-in-gate.cy.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { disableCMP } from '../../lib/disableCMP';
import { disableCMP } from '../../lib/disableCMP.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { Standard } from '../../../fixtures/generated/articles/Standard';
import { Standard } from '../../../fixtures/generated/articles/Standard.ts';
/* eslint-disable no-undef */
/* eslint-disable func-names */

Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/cypress/e2e/parallel-2/lightbox.cy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-undef */
/* eslint-disable func-names */
import { mockApi } from '../../lib/mocks';
import { disableCMP } from '../../lib/disableCMP';
import { mockApi } from '../../lib/mocks.js';
import { disableCMP } from '../../lib/disableCMP.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';

const articleUrl =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-undef */
/* eslint-disable func-names */
import { mockApi } from '../../lib/mocks';
import { disableCMP } from '../../lib/disableCMP';
import { mockApi } from '../../lib/mocks.js';
import { disableCMP } from '../../lib/disableCMP.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';

const READER_REVENUE_TITLE_TEXT = 'Support the';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-undef */
/* eslint-disable func-names */
import { storage } from '@guardian/libs';
import { mockApi } from '../../lib/mocks';
import { mockApi } from '../../lib/mocks.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { disableCMP } from '../../lib/disableCMP.js';

Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/cypress/e2e/parallel-4/signedin.cy.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable no-undef */
/* eslint-disable func-names */
import { disableCMP } from '../../lib/disableCMP';
import { disableCMP } from '../../lib/disableCMP.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { Standard } from '../../../fixtures/generated/articles/Standard';

Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/cypress/e2e/parallel-5/atom.video.cy.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { cmpIframe } from '../../lib/cmpIframe';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe';
import { cmpIframe } from '../../lib/cmpIframe.js';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe.js';
import { storage } from '@guardian/libs';

const interceptPlayEvent = ({ id, times = 1 }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
/* eslint-disable mocha/no-skipped-tests */
/* eslint-disable no-undef */
/* eslint-disable func-names */
import { mockApi } from '../../lib/mocks';
import { disableCMP } from '../../lib/disableCMP';
import { mockApi } from '../../lib/mocks.js';
import { disableCMP } from '../../lib/disableCMP.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { tweetBlock } from '../../fixtures/manual/tweet-block';
import { match1, match2 } from '../../fixtures/manual/cricket-match';
import { tweetBlock } from '../../fixtures/manual/tweet-block.js';
import { match1, match2 } from '../../fixtures/manual/cricket-match.js';

const blogUrl =
'https://www.theguardian.com/australia-news/live/2022/feb/22/australia-news-live-updates-scott-morrison-nsw-trains-coronavirus-covid-omicron-weather';
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/cypress/e2e/parallel-6/commercial.cy.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { cmpIframe } from '../../lib/cmpIframe.js';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe.js';
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { storage } from '@guardian/libs';

Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/cypress/e2e/parallel-6/consent.cy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable mocha/no-setup-in-describe */
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { cmpIframe } from '../../lib/cmpIframe';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe';
import { cmpIframe } from '../../lib/cmpIframe.js';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe.js';
import { storage } from '@guardian/libs';

const firstPage =
Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/cypress/e2e/parallel-6/paid.content.cy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable mocha/no-setup-in-describe */
import { setLocalBaseUrl } from '../../lib/setLocalBaseUrl.js';
import { cmpIframe } from '../../lib/cmpIframe';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe';
import { cmpIframe } from '../../lib/cmpIframe.js';
import { privacySettingsIframe } from '../../lib/privacySettingsIframe.js';
import { storage } from '@guardian/libs';

const handleCommercialErrors = () => {
Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/cypress/lib/mocks.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mostRead } from '../fixtures/manual/most-read';
import { mostReadGeo } from '../fixtures/manual/most-read-geo';
import { mostRead } from '../fixtures/manual/most-read.js';
import { mostReadGeo } from '../fixtures/manual/most-read-geo.js';

export const mockApi = () => {
// Mock share count
Expand Down
3 changes: 1 addition & 2 deletions dotcom-rendering/cypress/support/e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
// https://on.cypress.io/configuration
// ***********************************************************

// Import commands.js using ES2015 syntax:
import './commands';
import './commands.js';
import 'cypress-plugin-tab';
import 'cypress-real-events';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ production build locally.
To do this:

$ make build
$ node dist/server.js
$ node dist/server.cjs

*Note, you will need AWS `frontend` credentials to run the service.*

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,23 @@
const swcConfig = require('./scripts/webpack/.swcrc.json');

/** @type {import('jest').Config} */
module.exports = {
testEnvironment: 'jsdom',
moduleDirectories: ['node_modules', 'src'],
moduleFileExtensions: [
'js',
'mjs',
'cts',
'cjs',
'jsx',
'ts',
'tsx',
'json',
'node',
],
extensionsToTreatAsEsm: ['.ts', '.tsx'],
transform: {
'^.+\\.(ts|tsx)$': ['@swc/jest', swcConfig],
'\\.[jt]sx?$': ['@swc/jest', swcConfig],
},
testMatch: ['**/*.test.+(ts|tsx|js)'],
setupFilesAfterEnv: ['<rootDir>/scripts/jest/setup.ts'],
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/lighthouserc.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module.exports = {
collect: {
url: [process.env.LHCI_URL],
startServerCommand:
'NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true node dist/server.js',
'NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true node dist/server.cjs',
numberOfRuns: '10',
puppeteerScript: './scripts/lighthouse/puppeteer-script.js',
settings: {
Expand Down
16 changes: 8 additions & 8 deletions dotcom-rendering/makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ export SHELL := /usr/bin/env bash
# messaging #########################################

define log
@node scripts/env/log $(1)
@node scripts/env/log.js $(1)
endef

define warn
@node scripts/env/log $(1) warn
@node scripts/env/log.js $(1) warn
endef

# deployment #########################################
Expand All @@ -30,7 +30,7 @@ build: clean-dist install
$(call log, "building production bundles")
@NODE_ENV=production webpack --config ./scripts/webpack/webpack.config.js --progress
$(call log, "generating Islands report card")
@node ./scripts/islands/island-descriptions.mjs
@node ./scripts/islands/island-descriptions.js


build-ci: clean-dist install
Expand All @@ -39,12 +39,12 @@ build-ci: clean-dist install

start-ci: install
$(call log, "starting PROD server...")
@NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true node dist/server.js
@NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true node dist/server.cjs

start: install
$(call log, "starting PROD server...")
@echo '' # just a spacer
NODE_ENV=production pm2 start dist/server.js
NODE_ENV=production pm2 start dist/server.cjs
@echo '' # just a spacer
$(call log, "PROD server is running")
@NODE_ENV=production pm2 logs
Expand All @@ -55,7 +55,7 @@ start: install
start-prod:
$(call log, "starting PROD server...")
@echo '' # just a spacer
NODE_ENV=production /usr/local/node/pm2 start dist/server.js
NODE_ENV=production /usr/local/node/pm2 start dist/server.cjs
@echo '' # just a spacer
$(call log, "PROD server is running")

Expand Down Expand Up @@ -102,11 +102,11 @@ storybook-dev: clear clean-dist install

cypress: clear clean-dist install build
$(call log, "starting PROD server for Cypress")
@NODE_ENV=production NODE_OPTIONS="--max-old-space-size=8192" DISABLE_LOGGING_AND_METRICS=true start-server-and-test 'node dist/server.js' 9000 'cypress run --spec "cypress/e2e/**/*"'
@NODE_ENV=production NODE_OPTIONS="--max-old-space-size=8192" DISABLE_LOGGING_AND_METRICS=true start-server-and-test 'node dist/server.cjs' 9000 'cypress run --spec "cypress/e2e/**/*"'

cypress-open: clear clean-dist install build
$(call log, "starting PROD server and opening Cypress")
@NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true start-server-and-test 'node dist/server.js' 9000 'cypress open --e2e --browser electron'
@NODE_ENV=production DISABLE_LOGGING_AND_METRICS=true start-server-and-test 'node dist/server.cjs' 9000 'cypress open --e2e --browser electron'

ampValidation: clean-dist install
$(call log, "starting AMP Validation test")
Expand Down
8 changes: 5 additions & 3 deletions dotcom-rendering/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"version": "0.1.0-alpha",
"license": "Apache-2.0",
"tocList": "README.md docs/contributing/**",
"type": "module",
"private": true,
"scripts": {
"lint": "yarn lint:src && yarn lint:cypress",
Expand All @@ -16,9 +17,9 @@
"prettier:fix": "prettier . --write --cache",
"lintstats": "yarn lint --format node_modules/eslint-stats/byError.js",
"tsc": "tsc",
"test": "jest --maxWorkers=50%",
"test:watch": "jest --watch --maxWorkers=25%",
"test:ci": "jest --runInBand",
"test": "NODE_OPTIONS=--experimental-vm-modules jest --maxWorkers=50%",
"test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch --maxWorkers=25%",
"test:ci": "NODE_OPTIONS=--experimental-vm-modules jest --runInBand",
"createtoc": "doctoc $npm_package_tocList --github --update-only --title '<!-- Automatically created with yarn run createtoc and on push hook -->' ",
"addandcommittoc": "git add $npm_package_tocList && git commit -m 'Add TOC update' || true",
"cypress:open": "cypress open",
Expand Down Expand Up @@ -82,6 +83,7 @@
"@guardian/source-react-components-development-kitchen": "14.0.0",
"@guardian/support-dotcom-components": "1.0.7",
"@guardian/tsconfig": "0.2.0",
"@jest/globals": "29.6.2",
"@sentry/browser": "7.37.2",
"@sentry/integrations": "7.37.2",
"@storybook/addon-essentials": "7.2.0",
Expand Down
Loading

0 comments on commit 84a4050

Please sign in to comment.