Skip to content

Commit

Permalink
fix(dev): merge 'assets' and 'output' paths
Browse files Browse the repository at this point in the history
- Remove the concept of the "assets" directory entirely, and set the
"output" directory to be just "web/".
- Remove the old logo asset from /web/ (pre-Venia rebrand!)
- Gitignore entire "web" directory
- Set JS files to output to 'web/js' subdirectory by adding directory
prefix to `output.filename` and `output.chunkFilename`
- Modify tests, documentation, and type expectations to remove
`paths.assets`
  • Loading branch information
zetlen committed Aug 10, 2018
1 parent d0e4e03 commit 36d8157
Show file tree
Hide file tree
Showing 17 changed files with 35 additions and 77 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
__fixtures__
dist
pwa-devdocs
packages/venia-concept/web
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ test-report.xml
test-results.json
lerna-debug.log
.env
packages/venia-concept/web
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ package-lock.json
dist
web/js
pwa-devdocs
packages/venia-concept/web
3 changes: 1 addition & 2 deletions packages/pwa-buildpack/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,7 @@ which reads an ini-formatted file to set the environment.
const themePaths = {
src: path.resolve(__dirname, 'src'),
assets: path.resolve(__dirname, 'web'),
output: path.resolve(__dirname, 'web/js')
output: path.resolve(__dirname, 'web')
};
```
Expand Down
3 changes: 1 addition & 2 deletions packages/pwa-buildpack/docs/PWADevServer.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ module.exports = async env => {
backendDomain: 'https://magento2.localdomain',
serviceWorkerFileName: 'sw.js',
paths: {
output: path.resolve(__dirname, 'web/js'),
assets: path.resolve(__dirname, 'web')
output: path.resolve(__dirname, 'web/'),
},
id: 'magento-venia'
})
Expand Down
7 changes: 3 additions & 4 deletions packages/pwa-buildpack/docs/ServiceWorkerPlugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ module.exports = async env => {
},

paths: {
output: path.resolve(__dirname, 'web/js'),
assets: path.resolve(__dirname, 'web')
output: path.resolve(__dirname, 'web')
},
enableServiceWorkerDebugging: true,
serviceWorkerFileName: 'sw.js',
Expand Down Expand Up @@ -63,9 +62,9 @@ Plugin constructor for the `ServiceWorkerPlugin` class.
`paths: Object` **(Required)**
The local absolute paths to theme folders.

- `paths.assets: String`
- `paths.output: String`

The directory for public static assets.
The directory for build output.

`enableServiceWorkerDebugging: Boolean`
When `true`, hot reloading is enabled and the ServiceWorker is active in the document root, regardless of the publicPath value.
Expand Down
3 changes: 1 addition & 2 deletions packages/pwa-buildpack/src/WebpackTools/PWADevServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const PWADevServer = {
publicPath: 'string',
backendDomain: 'string',
'paths.output': 'string',
'paths.assets': 'string',
serviceWorkerFileName: 'string'
}),
hostnamesById: new GlobalConfig({
Expand Down Expand Up @@ -165,7 +164,7 @@ const PWADevServer = {
},
after(app) {
// set static server to load and serve from different paths
app.use(config.publicPath, express.static(config.paths.assets));
app.use(config.publicPath, express.static(config.paths.output));

// proxy to backend
app.use(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,15 @@ test('.configure() throws errors on missing config', async () => {
id: 'foo',
publicPath: 'bar',
backendDomain: 'https://dumb.domain',
paths: { output: 'output' }
paths: { output: 1234 }
})
).rejects.toThrow('paths.assets must be of type string');
).rejects.toThrow('paths.output must be of type string');
await expect(
PWADevServer.configure({
id: 'foo',
publicPath: 'bar',
backendDomain: 'https://dumb.domain',
paths: { output: 'foo', assets: 'bar' }
paths: { output: 'foo' }
})
).rejects.toThrow('serviceWorkerFileName must be of type string');
});
Expand All @@ -243,8 +243,7 @@ test('.configure() gets or creates an SSL cert', async () => {
const server = await PWADevServer.configure({
id: 'heckin',
paths: {
output: 'good',
assets: 'boye'
output: 'good'
},
publicPath: 'bork',
serviceWorkerFileName: 'doin',
Expand All @@ -267,8 +266,7 @@ test('.configure() returns a configuration object for the `devServer` property o
const config = {
id: 'Theme_Unique_Id',
paths: {
output: 'path/to/static',
assets: 'path/to/assets'
output: 'path/to/static'
},
publicPath: 'full/path/to/publicPath',
serviceWorkerFileName: 'swname.js',
Expand Down Expand Up @@ -307,8 +305,7 @@ test('.configure() returns a configuration object with before() and after() hand
const config = {
id: 'Theme_Unique_Id',
paths: {
output: 'path/to/static',
assets: 'path/to/assets'
output: 'path/to/static'
},
publicPath: 'full/path/to/publicPath',
serviceWorkerFileName: 'swname.js',
Expand Down Expand Up @@ -364,8 +361,7 @@ test('.configure() optionally adds OriginSubstitution middleware', async () => {
const config = {
id: 'Theme_Unique_Id',
paths: {
output: 'path/to/static',
assets: 'path/to/assets'
output: 'path/to/static'
},
publicPath: 'full/path/to/publicPath',
serviceWorkerFileName: 'swname.js',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class ServiceWorkerPlugin {
static validateOptions = optionsValidator('ServiceWorkerPlugin', {
'env.phase': 'string',
serviceWorkerFileName: 'string',
'paths.assets': 'string'
'paths.output': 'string'
});
constructor(config) {
ServiceWorkerPlugin.validateOptions('ServiceWorkerPlugin', config);
Expand All @@ -17,7 +17,7 @@ class ServiceWorkerPlugin {
const config = {
// `globDirectory` and `globPatterns` must match at least 1 file
// otherwise workbox throws an error
globDirectory: this.config.paths.assets,
globDirectory: this.config.paths.output,
// TODO: (feature) autogenerate glob patterns from asset manifest
globPatterns: ['**/*.{gif,jpg,png,svg}'],

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ test('throws if options are missing', () => {
env: { phase: 'development' },
serviceWorkerFileName: 'file.name'
})
).toThrow('paths.assets must be of type string');
).toThrow('paths.output must be of type string');
});

test('returns a valid Webpack plugin', () => {
Expand All @@ -31,7 +31,7 @@ test('returns a valid Webpack plugin', () => {
serviceWorkerFileName: 'sw.js',
runtimeCacheAssetPath: 'https://location/of/assets',
paths: {
assets: 'path/to/assets'
output: 'path/to/assets'
}
})
).toHaveProperty('apply', expect.any(Function));
Expand All @@ -45,7 +45,7 @@ test('.apply calls WorkboxPlugin.GenerateSW in prod', () => {
serviceWorkerFileName: 'sw.js',
runtimeCacheAssetPath: 'https://location/of/assets',
paths: {
assets: 'path/to/assets'
output: 'path/to/assets'
}
});
const workboxApply = jest.fn();
Expand Down Expand Up @@ -75,7 +75,7 @@ test('.apply calls nothing but warns in console in dev', () => {
serviceWorkerFileName: 'sw.js',
runtimeCacheAssetPath: 'https://location/of/assets',
paths: {
assets: 'path/to/assets'
output: 'path/to/assets'
}
});
jest.spyOn(console, 'warn').mockImplementationOnce(() => {});
Expand Down Expand Up @@ -103,7 +103,7 @@ test('.apply generates and writes out a serviceworker when enableServiceWorkerDe
serviceWorkerFileName: 'sw.js',
runtimeCacheAssetPath: 'https://location/of/assets',
paths: {
assets: 'path/to/assets'
output: 'path/to/assets'
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,12 @@ const {

Add the following content to `webpack.config.js` to define the paths to your theme resources:

``` javascript
``` javascript
const path = require('path');

const themePaths = {
src: path.resolve(__dirname, 'src'),
assets: path.resolve(__dirname, 'web'),
output: path.resolve(__dirname, 'web/js'),
output: path.resolve(__dirname, 'web'),
};
```

Expand All @@ -132,8 +131,8 @@ module.exports = async function(env) {
output: {
path: themePaths.output,
publicPath: process.env.MAGENTO_BACKEND_PUBLIC_PATH,
filename: '[name].js',
chunkFilename: '[name].js'
filename: 'js/[name].js',
chunkFilename: 'js/[name]-[chunkhash].js'
},
module: {
rules: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ const path = require('path');

const themePaths = {
src: path.resolve(__dirname, 'src'),
assets: path.resolve(__dirname, 'web'),
output: path.resolve(__dirname, 'web/js'),
output: path.resolve(__dirname, 'web')
};

module.exports = async function(env) {
Expand All @@ -34,8 +33,8 @@ module.exports = async function(env) {
output: {
path: themePaths.output,
publicPath: process.env.MAGENTO_BACKEND_PUBLIC_PATH,
filename: '[name].js',
chunkFilename: '[name].js'
filename: 'js/[name].js',
chunkFilename: 'js/[name]-[chunkhash].js'
},
module: {
rules: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ This is a list of common object types shared between the PWA-Buildpack modules.
| ---------------- | ---------------------------------------------------------------------------- |
| `root: string` | The absolute path of the project's root directory on the working filesystem. |
| `output: string` | The directory where webpack should output any built assets. |
| `assets: string` | The directory where any assets not explicitly build by webpack is found. |
{:style="table-layout:auto"}

### Output location
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ module.exports = async env => {
backendDomain: 'https://magento2.localdomain',
serviceWorkerFileName: 'sw.js',
paths: {
output: path.resolve(__dirname, 'web/js'),
assets: path.resolve(__dirname, 'web')
output: path.resolve(__dirname, 'web')
},
id: 'magento-venia'
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ module.exports = async env => {
},

paths: {
output: path.resolve(__dirname, 'web/js'),
assets: path.resolve(__dirname, 'web')
output: path.resolve(__dirname, 'web')
},
enableServiceWorkerDebugging: true,
serviceWorkerFileName: 'sw.js',
Expand Down
Loading

0 comments on commit 36d8157

Please sign in to comment.