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

Move the MRT reference app to the SDKs, so that we can verify eg. Node support #966

Merged
merged 16 commits into from
Mar 6, 2023
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
3 changes: 2 additions & 1 deletion packages/pwa-kit-create-app/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ const main = () => {
const pkgNames = [
'template-retail-react-app',
'template-express-minimal',
'template-typescript-minimal'
'template-typescript-minimal',
'template-mrt-reference-app'
]

if (!sh.test('-d', templatesDir)) {
Expand Down
10 changes: 9 additions & 1 deletion packages/pwa-kit-create-app/scripts/create-mobify-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ const EXPRESS_MINIMAL = 'express-minimal'
const TEST_PROJECT = 'test-project' // TODO: This will be replaced with the `isomorphic-client` config.
const RETAIL_REACT_APP_DEMO = 'retail-react-app-demo'
const RETAIL_REACT_APP = 'retail-react-app'
const MRT_REFERENCE_APP = 'mrt-reference-app'

const PRIVATE_PRESETS = [
TEST_PROJECT,
EXPRESS_MINIMAL_TEST_PROJECT,
TYPESCRIPT_MINIMAL_TEST_PROJECT
TYPESCRIPT_MINIMAL_TEST_PROJECT,
MRT_REFERENCE_APP
]
const PUBLIC_PRESETS = [
RETAIL_REACT_APP_DEMO,
Expand Down Expand Up @@ -464,6 +466,12 @@ const main = (opts) => {
})
case TEST_PROJECT:
return runGenerator(testProjectAnswers(), opts)
case MRT_REFERENCE_APP:
return runTemplateGenerator(
'mrt-reference-app',
opts,
'template-mrt-reference-app'
)
case RETAIL_REACT_APP_DEMO:
return Promise.resolve()
.then(() => runGenerator(demoProjectAnswers(), opts))
Expand Down
10 changes: 4 additions & 6 deletions packages/pwa-kit-dev/src/configs/webpack/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,12 +366,6 @@ const renderer =
excludeWarnings: true,
skipFirstNotification: true
}),

// Must only appear on one config – this one is the only mandatory one.
new CopyPlugin({
patterns: [{from: 'app/static/', to: 'static/'}]
}),

analyzeBundle && getBundleAnalyzerPlugin('server-renderer')
].filter(Boolean)
}
Expand All @@ -395,6 +389,10 @@ const ssr = (() => {
},
plugins: [
...config.plugins,
// This must only appear on one config – this one is the only mandatory one.
new CopyPlugin({
patterns: [{from: 'app/static/', to: 'static/'}]
}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out, it's this config that is really always mandatory. My bad!

analyzeBundle && getBundleAnalyzerPlugin(SSR)
].filter(Boolean)
}
Expand Down
53 changes: 22 additions & 31 deletions packages/pwa-kit-dev/src/ssr/server/build-dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import {
REQUEST_PROCESSOR
} from '../../configs/webpack/config-names'
import {randomUUID} from 'crypto'
const projectDir = process.cwd()
const projectWebpackPath = path.resolve(projectDir, 'webpack.config.js')

const chalk = require('chalk')

Expand Down Expand Up @@ -125,6 +123,8 @@ export const DevServerMixin = {
// But the SSR render function must!

let config = require('../../configs/webpack/config')

const projectWebpackPath = path.resolve(app.options.projectDir, 'webpack.config.js')
if (fs.existsSync(projectWebpackPath)) {
config = require(projectWebpackPath)
}
Expand Down Expand Up @@ -214,36 +214,27 @@ export const DevServerMixin = {
},

serveStaticFile(filePath, opts = {}) {
// Warning: Ugly part of the Bundle spec that we need to maintain.
//
// This function assumes that an SDK build step will copy all
// non-webpacked assets from the 'app' dir to the 'build' dir.
//
// If you look carefully through the history, this has never
// been true though – assets get copied from app/static to
// build/static but this isn't really clear from the API.
//
// To see where those assets get copied, see here:
//
// packages/pwa-kit-dev/src/configs/webpack/config.js
//
// We have plans to make a robust Bundle spec in 246!
//
// Discussion here:
//
// https://salesforce-internal.slack.com/archives/C8YDDMKFZ/p1677793769255659?thread_ts=1677791840.174309&cid=C8YDDMKFZ
return (req, res) => {
req.app.__devMiddleware.waitUntilValid(() => {
const options = req.app.options
const webpackStats = req.app.__devMiddleware.context.stats.stats

const serverCompilation = webpackStats.find(
// static files are copied into bundle
// in the server webpack config
(stat) => stat.compilation.name === SERVER
).compilation
const {assetsInfo} = serverCompilation
const assetInfo = assetsInfo.get(filePath)

// if the asset is not in the webpack bundle, then
// return 404, we don't care whether or not the file
// exists in the local file system
if (!assetInfo) {
res.sendStatus(404)
return
}
const {sourceFilename} = assetInfo
const sourceFilePath = path.resolve(sourceFilename)

res.sendFile(sourceFilePath, {
headers: {
'cache-control': options.defaultCacheControl
},
...opts
})
})
const baseDir = path.resolve(req.app.options.projectDir, 'app')
return this._serveStaticFile(req, res, baseDir, filePath, opts)
}
},

Expand Down
41 changes: 3 additions & 38 deletions packages/pwa-kit-dev/src/ssr/server/build-dev-server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -738,59 +738,24 @@ describe('DevServer service worker', () => {

test(`${name} (and handle 404s correctly)`, () => {
const app = createApp()
app.get('/worker.js(.map)?', NoWebpackDevServerFactory.serveServiceWorker)
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 test had a bug where it didn't actually run serveServiceWorker on a 404, so it missed a branch in the coverage.


return request(app).get(requestPath).expect(404)
})
})
})

describe('DevServer serveStaticFile', () => {
// This isn't ideal! We need a way to test the dev middleware
// including the on demand webpack compiler. However, the webpack config and
// the Dev server assumes the code runs at the root of a project.
// When we run the tests, we are not in a project.
// We have a /test_fixtures project, but Jest does not support process.chdir(),
// nor mocking process.cwd(), so we mock the dev middleware for now.
// TODO: create a proper testing fixture project and run the tests in the isolated
// project environment.
const MockWebpackDevMiddleware = {
waitUntilValid: (cb) => cb(),
context: {
stats: {
stats: [
{
compilation: {
name: 'server',
assetsInfo: new Map([
[
'static/favicon.ico',
{
sourceFilename: path.resolve(
testFixtures,
'app/static/favicon.ico'
)
}
]
])
}
}
]
}
}
}

test('should serve static files', async () => {
const options = opts()
const options = opts({projectDir: testFixtures})
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 made this configurable, so we don't have to do this middleware mocking.

const app = NoWebpackDevServerFactory._createApp(options)
app.__devMiddleware = MockWebpackDevMiddleware
app.use('/test', NoWebpackDevServerFactory.serveStaticFile('static/favicon.ico'))
return request(app).get('/test').expect(200)
})

test('should return 404 if static file does not exist', async () => {
const options = opts()
const options = opts({projectDir: testFixtures})
const app = NoWebpackDevServerFactory._createApp(options)
app.__devMiddleware = MockWebpackDevMiddleware
app.use('/test', NoWebpackDevServerFactory.serveStaticFile('static/IDoNotExist.ico'))
return request(app).get('/test').expect(404)
})
Expand Down
27 changes: 19 additions & 8 deletions packages/pwa-kit-runtime/src/ssr/server/build-remote-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ export const RemoteServerFactory = {
* testing, or to handle non-standard projects.
*/
const defaults = {
// For test only – allow the project dir to be overridden.
projectDir: process.cwd(),

// Absolute path to the build directory
buildDir: path.resolve(process.cwd(), BUILD),

Expand Down Expand Up @@ -741,17 +744,25 @@ export const RemoteServerFactory = {
*/
serveStaticFile(filePath, opts = {}) {
return (req, res) => {
const options = req.app.options
const file = path.resolve(options.buildDir, filePath)
res.sendFile(file, {
headers: {
[CACHE_CONTROL]: options.defaultCacheControl
},
...opts
})
const baseDir = req.app.options.buildDir
return this._serveStaticFile(req, res, baseDir, filePath, opts)
}
},

/**
* @private
*/
_serveStaticFile(req, res, baseDir, filePath, opts = {}) {
const options = req.app.options
const file = path.resolve(baseDir, filePath)
res.sendFile(file, {
headers: {
[CACHE_CONTROL]: options.defaultCacheControl
},
...opts
})
},

/**
* Server side rendering entry.
*
Expand Down
7 changes: 7 additions & 0 deletions packages/template-mrt-reference-app/.eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
build
coverage
docs
app/static
jest.config.js
webpack
scripts/generator/assets
9 changes: 9 additions & 0 deletions packages/template-mrt-reference-app/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright (c) 2021, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
module.exports = {
extends: require.resolve('pwa-kit-dev/configs/eslint/eslint-config')
}
3 changes: 3 additions & 0 deletions packages/template-mrt-reference-app/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
build
node_modules
build.tar
4 changes: 4 additions & 0 deletions packages/template-mrt-reference-app/.prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
build
docs
coverage
scripts/generator/assets
7 changes: 7 additions & 0 deletions packages/template-mrt-reference-app/.prettierrc.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
printWidth: 100
singleQuote: true
semi: false
bracketSpacing: false
tabWidth: 4
arrowParens: 'always'
trailingComma: 'none'
14 changes: 14 additions & 0 deletions packages/template-mrt-reference-app/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
BSD 3-Clause License

Copyright (c) 2021, Salesforce.com, Inc.
All rights reserved.

Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:

1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.

2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.

3. Neither the name of Salesforce.com nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
25 changes: 25 additions & 0 deletions packages/template-mrt-reference-app/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# template-mrt-reference-app

This is the reference app that the Managed Runtime Team uses to test
features in the platform (eg. TLS versions, successful deploys, proxy
behaviour).

This app is intended to be a thin layer over the bare minimum SDKs
that we expect/require all MRT users to use.

Although MRT started life primarily as a hosting environment for
React apps, we're expanding that to support other technologies –
this app lets us test those platform features that are universal
across all apps, regardless of framework choice.


## Usage in CI/CD tests ⛅️

This app is deployed to several pre-existing test Targets as part
of a "smoke-test" of the MRT platform. To see the Targets in use
take a look at the CI config in

https://git.soma.salesforce.com/cc-mobify/ssr-infrastructure/blob/sfci-main/Jenkinsfile#L176

These smoke-tests are triggered by merges to the main development
branch of the above repository.
35 changes: 35 additions & 0 deletions packages/template-mrt-reference-app/app/request-processor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2023, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import {QueryParameters} from 'pwa-kit-runtime/utils/ssr-request-processing'

const exclusions = ['removeme']

export const processRequest = ({path, querystring, parameters}) => {
console.assert(parameters, 'Missing parameters')

// Query string filtering

// Build a first QueryParameters object from the given querystring
const queryParameters = new QueryParameters(querystring)

// Build a second QueryParameters from the first, with all
// excluded parameters removed
const filtered = QueryParameters.from(
queryParameters.parameters.filter(
// parameter.key is always lower-case
(parameter) => !exclusions.includes(parameter.key)
)
)

// Re-generate the querystring
querystring = filtered.toString()

return {
path,
querystring
}
}
Loading