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.

Sign up for GitHub

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

chore: Misc cleaning #1659

Merged
merged 4 commits into from
Feb 9, 2022
Merged

chore: Misc cleaning #1659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5ae032
chore: Misc cleaning
rschristian Feb 7, 2022
b9cfb78
fix: Correcting push-manifest in non-ESM builds
rschristian Feb 8, 2022
ac53c63
test: Adding TS template to build suite
rschristian Feb 8, 2022
00a3cd3
docs: Adding changeset
rschristian Feb 8, 2022
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
5 changes: 5 additions & 0 deletions .changeset/few-pears-push.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'preact-cli': patch
---

Corrects `push-manifest.json` generation in non-ESM builds
41 changes: 0 additions & 41 deletions packages/cli/lib/lib/output-hooks.js
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions packages/cli/lib/lib/webpack/create-load-manifest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = (assets, isESMBuild = false, namedChunkGroups) => {
styles = [];
for (let filename in assets) {
if (!/\.map$/.test(filename)) {
if (/route-/.test(filename)) {
if (/^route-.*\.js$/.test(filename)) {
Copy link
Member Author

@rschristian rschristian Feb 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This created some junk output in push-manifest.json when ESM is disabled, like in our test suite. Wouldn't have ever been a problem, as these junk "routes" would never match to a URL when building the HTML, but certainly looked odd if any user took a peek.

{
  "/": {
    "bundle.2da73.css": {
      "type": "style",
      "weight": 1
    },
    "bundle.44866.js": {
      "type": "script",
      "weight": 1
    },
    "route-home.chunk.3cec8.js": {
      "type": "script",
      "weight": 0.9
    },
    "route-home.chunk.bcb8a.css": {
      "type": "style",
      "weight": 0.9
    }
  },
  "/.chunk.bcb8a.css": {
    "bundle.2da73.css": {
      "type": "style",
      "weight": 1
    },
    "bundle.44866.js": {
      "type": "script",
      "weight": 1
    },
    "route-home.chunk.bcb8a.css": {
      "type": "script",
      "weight": 0.9
    }
  },
  "/profile.chunk.6dd80.css": {
    "bundle.2da73.css": {
      "type": "style",
      "weight": 1
    },
    "bundle.44866.js": {
      "type": "script",
      "weight": 1
    },
    "route-profile.chunk.6dd80.css": {
      "type": "script",
      "weight": 0.9
    }
  },
  "/profile": {
    "bundle.2da73.css": {
      "type": "style",
      "weight": 1
    },
    "bundle.44866.js": {
      "type": "script",
      "weight": 1
    },
    "route-profile.chunk.ddf94.js": {
      "type": "script",
      "weight": 0.9
    },
    "route-profile.chunk.6dd80.css": {
      "type": "style",
      "weight": 0.9
    }
  }
}

// both ESM & regular match here
isMatch(filename, isESMBuild) && scripts.push(filename);
} else if (/chunk\.(.+)\.css$/.test(filename)) {
Expand Down Expand Up @@ -39,8 +39,8 @@ module.exports = (assets, isESMBuild = false, namedChunkGroups) => {
};

let path, css, obj;
scripts.forEach((filename, idx) => {
css = styles[idx];
scripts.forEach(filename => {
css = styles.find(asset => asset.startsWith(filename.replace(/\..*/, '')));
obj = Object.assign({}, defaults);
obj[filename] = { type: 'script', weight: 0.9 };
if (css) obj[css] = { type: 'style', weight: 0.9 };
Expand Down
48 changes: 21 additions & 27 deletions packages/cli/lib/lib/webpack/webpack-client-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const cleanFilename = name =>
* @returns {Promise<import('webpack').Configuration>}
*/
async function clientConfig(env) {
const { isProd, source, src, cwd /*, port? */ } = env;
const { source, src, cwd } = env;
const IS_SOURCE_PREACT_X_OR_ABOVE = isInstalledVersionPreactXOrAbove(cwd);
const asyncLoader = IS_SOURCE_PREACT_X_OR_ABOVE
? require.resolve('@preact/async-loader')
Expand Down Expand Up @@ -84,7 +84,7 @@ async function clientConfig(env) {
// copy any static files
existsSync(source('assets')) && { from: 'assets', to: 'assets' },
// copy sw-debug
!isProd && {
!env.isProd && {
from: resolve(__dirname, '../../resources/sw-debug.js'),
to: 'sw-debug.js',
},
Expand All @@ -100,7 +100,7 @@ async function clientConfig(env) {
output: {
path: env.dest,
publicPath: '/',
filename: isProd ? '[name].[chunkhash:5].js' : '[name].js',
filename: env.isProd ? '[name].[chunkhash:5].js' : '[name].js',
chunkFilename: '[name].chunk.[chunkhash:5].js',
},

Expand Down Expand Up @@ -143,6 +143,8 @@ async function clientConfig(env) {
plugins: [
new webpack.DefinePlugin({
'process.env.ES_BUILD': false,
'process.env.ADD_SW': env.sw,
'process.env.PRERENDER': env.prerender,
}),
new PushManifestPlugin(env),
...(await renderHTMLPlugin(env)),
Expand All @@ -156,14 +158,12 @@ async function clientConfig(env) {
};
}

function getBabelEsmPlugin(config) {
Copy link
Member Author

@rschristian rschristian Feb 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Besides a few spots in client-config, everywhere else we use env, rather than config. This has been slightly awkward to follow as I've done v5 stuff.

function getBabelEsmPlugin(env) {
const esmPlugins = [];
if (config.esm) {
if (env.esm) {
esmPlugins.push(
new BabelEsmPlugin({
filename: config.isProd
? '[name].[chunkhash:5].esm.js'
: '[name].esm.js',
filename: env.isProd ? '[name].[chunkhash:5].esm.js' : '[name].esm.js',
chunkFilename: '[name].chunk.[chunkhash:5].esm.js',
excludedPlugins: ['BabelEsmPlugin', 'InjectManifest'],
beforeStartExecution: plugins => {
Expand Down Expand Up @@ -196,7 +196,7 @@ function getBabelEsmPlugin(config) {
/**
* @returns {import('webpack').Configuration}
*/
function isProd(config) {
function isProd(env) {
let limit = 200 * 1000; // 200kb
const prodConfig = {
performance: Object.assign(
Expand All @@ -205,14 +205,12 @@ function isProd(config) {
maxAssetSize: limit,
maxEntrypointSize: limit,
},
config.pkg.performance
env.pkg.performance
),

plugins: [
new webpack.DefinePlugin({
'process.env.ADD_SW': config.sw,
'process.env.ESM': config.esm,
'process.env.PRERENDER': config.prerender,
'process.env.ESM': env.esm,
}),
new SizePlugin(),
],
Expand Down Expand Up @@ -252,7 +250,7 @@ function isProd(config) {
},
};

if (config['inline-css']) {
if (env['inline-css']) {
prodConfig.plugins.push(
new CrittersPlugin({
preload: 'media',
Expand All @@ -263,11 +261,11 @@ function isProd(config) {
);
}

if (config.analyze) {
if (env.analyze) {
prodConfig.plugins.push(new BundleAnalyzerPlugin());
}

if (config.brotli) {
if (env.brotli) {
prodConfig.plugins.push(
new CompressionPlugin({
filename: '[path].br[query]',
Expand All @@ -283,21 +281,17 @@ function isProd(config) {
/**
* @returns {import('webpack').Configuration}
*/
function isDev(config) {
const { cwd, src, refresh } = config;
function isDev(env) {
const { cwd, src } = env;

return {
infrastructureLogging: {
level: 'info',
},
plugins: [
new webpack.NamedModulesPlugin(),
...(refresh ? [new RefreshPlugin()] : []),
new webpack.DefinePlugin({
'process.env.ADD_SW': config.sw,
'process.env.PRERENDER': config.prerender,
}),
Comment on lines -296 to -299
Copy link
Member Author

Choose a reason for hiding this comment

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

Values are duplicated with prod config, so they can be moved up to common.

],
env.refresh && new RefreshPlugin(),
].filter(Boolean),

devServer: {
hot: true,
Expand All @@ -312,9 +306,9 @@ function isDev(config) {
ignored: [resolve(cwd, 'build'), resolve(cwd, 'node_modules')],
},
},
https: config.https,
port: config.port,
host: process.env.HOST || config.host || '0.0.0.0',
https: env.https,
port: env.port,
host: process.env.HOST || env.host || '0.0.0.0',
allowedHosts: 'all',
historyApiFallback: true,
client: {
Expand Down
50 changes: 37 additions & 13 deletions packages/cli/tests/build.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
const { join } = require('path');
const { existsSync } = require('fs');
const { readFile } = require('fs').promises;
const { access, readdir, readFile } = require('fs').promises;
const looksLike = require('html-looks-like');
const { create, build } = require('./lib/cli');
const { snapshot } = require('./lib/utils');
const { subject } = require('./lib/output');
const images = require('./images/build');
const { promisify } = require('util');
const glob = promisify(require('glob').glob);
const minimatch = require('minimatch');
const shell = require('shelljs');

const prerenderUrlFiles = [
'prerender-urls.json',
Expand Down Expand Up @@ -69,6 +67,17 @@ describe('preact build', () => {
testMatch(output, images['default-esm']);
});

it(`builds the 'typescript' template`, async () => {
let dir = await create('typescript');

// The tsconfig.json in the template covers the test directory,
// so TS will error out if it can't find even test-only module definitions
shell.cd(dir);
shell.exec('npm i @types/enzyme enzyme-adapter-preact-pure');

expect(() => build(dir)).not.toThrow();
});

it('should use SASS styles', async () => {
let dir = await subject('sass');
await build(dir);
Expand All @@ -79,12 +88,17 @@ describe('preact build', () => {

it('should use custom `.babelrc`', async () => {
// app with custom .babelrc enabling async functions
let app = await subject('custom-babelrc');
let dir = await subject('custom-babelrc');

await build(app);
await build(dir);

const bundleFiles = await glob(`${app}/build/bundle.*.js`);
const transpiledChunk = await readFile(bundleFiles[0], 'utf8');
const bundleFile = (await readdir(`${dir}/build`)).find(file =>
/bundle\.\w{5}\.js$/.test(file)
);
const transpiledChunk = await readFile(
`${dir}/build/${bundleFile}`,
'utf8'
);

// when tragetting only last 1 chrome version, babel preserves
// arrow function. So checking for the delay function code from delay function in
Expand Down Expand Up @@ -183,8 +197,8 @@ describe('preact build', () => {
let dir = await subject('custom-webpack');
await build(dir);

let file = join(dir, 'build/bundle.js');
expect(existsSync(file)).toBe(true);
let stableOutput = join(dir, 'build/bundle.js');
expect(await access(stableOutput)).toBeUndefined();
});

it('should use custom `template.html`', async () => {
Expand Down Expand Up @@ -218,16 +232,26 @@ describe('preact build', () => {
let dir = await subject('static-root');
await build(dir);
let file = join(dir, 'build', '.htaccess');
expect(existsSync(file)).toBe(true);
expect(await access(file)).toBeUndefined();
});

it('should error out for invalid argument', async () => {
it('should error out for invalid CLI argument', async () => {
let dir = await subject('custom-template');
const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {});
expect(build(dir, { 'service-worker': false })).rejects.toEqual(
await expect(build(dir, { 'service-worker': false })).rejects.toEqual(
new Error('Invalid argument found.')
);
expect(mockExit).toHaveBeenCalledWith(1);
mockExit.mockRestore();
});

it('should produce correct push-manifest', async () => {
let dir = await create('default');

await build(dir);
const manifest = await readFile(`${dir}/build/push-manifest.json`, 'utf8');
expect(manifest).toEqual(
expect.stringMatching(getRegExpFromMarkup(images.pushManifest))
);
});
});
25 changes: 0 additions & 25 deletions packages/cli/tests/client.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,6 @@ describe('client-side tests', () => {
PORT = await getPort();
});

it.skip('should hydrate routes progressively.', async () => {
let dir = await subject('progressive-hydration');
await build(dir);
const server = getServer(join(dir, 'build'), PORT);

// let page = await loadPage(chrome, `http://127.0.0.1:${PORT}/`);
const page = await chrome.newPage();

page.on('console', consoleMessage => {
// eslint-disable-next-line
console[consoleMessage.type()](consoleMessage.text());
});

await page.goto(`http://127.0.0.1:${PORT}/`);

// await waitUntilExpression(page, `window.booted`);
await sleep(500);

const mutations = await page.evaluate('window.mutations');

expect(mutations).toHaveLength(0);

server.server.close();
});

Comment on lines -25 to -49
Copy link
Member Author

Choose a reason for hiding this comment

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

Has been skipped since it was added: #957

it('should hydrate routes progressively with preact8.', async () => {
let dir = await subject('progressive-hydration-preact8');
await build(dir, {}, true);
Expand Down
43 changes: 42 additions & 1 deletion packages/cli/tests/images/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ exports.default = Object.assign({}, common, {
'index.html': 2034,
'manifest.json': 455,
'preact_prerender_data.json': 11,
'push-manifest.json': 812,
'push-manifest.json': 450,
'route-home.chunk.bcb8a.css': 58,
'route-home.chunk.3cec8.js': 327,
'route-home.chunk.3cec8.js.map': 483,
Expand Down Expand Up @@ -212,3 +212,44 @@ exports.template = `
</body>
</html>
`;

exports.pushManifest = `
{
"/":{
"bundle.\\w{5}.css":{
"type":"style",
"weight":1
},
"bundle.\\w{5}.js":{
"type":"script",
"weight":1
},
"route-home.chunk.\\w{5}.js":{
"type":"script",
"weight":0.9
},
"route-home.chunk.\\w{5}.css":{
"type":"style",
"weight":0.9
}
},
"/profile":{
"bundle.\\w{5}.css":{
"type":"style",
"weight":1
},
"bundle.\\w{5}.js":{
"type":"script",
"weight":1
},
"route-profile.chunk.\\w{5}.js":{
"type":"script",
"weight":0.9
},
"route-profile.chunk.\\w{5}.css":{
"type":"style",
"weight":0.9
}
}
}
`;
Loading