Skip to content

Commit

Permalink
Don't let deepmerge merge instantiated objects (#1534)
Browse files Browse the repository at this point in the history
* feat: don't deepmerge non-plain objects

Fixes #1416

* test: add new tests/snapshots

One for js config files, and one for instantiated objects
  • Loading branch information
tivac authored Nov 8, 2020
1 parent fec4f57 commit 0b3813f
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 2 deletions.
1 change: 1 addition & 0 deletions snowpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"glob": "^7.1.4",
"http-proxy": "^1.18.1",
"httpie": "^1.1.2",
"is-plain-object": "^5.0.0",
"isbinaryfile": "^4.0.6",
"jsonschema": "~1.2.5",
"kleur": "^4.1.1",
Expand Down
9 changes: 7 additions & 2 deletions snowpack/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {cosmiconfigSync} from 'cosmiconfig';
import {all as merge} from 'deepmerge';
import * as esbuild from 'esbuild';
import http from 'http';
import {isPlainObject} from 'is-plain-object';
import {validate, ValidatorResult} from 'jsonschema';
import os from 'os';
import path from 'path';
Expand Down Expand Up @@ -855,7 +856,9 @@ export function createConfiguration(
if (validationErrors.length > 0) {
return [validationErrors, undefined];
}
const mergedConfig = merge<SnowpackConfig>([DEFAULT_CONFIG, config]);
const mergedConfig = merge<SnowpackConfig>([DEFAULT_CONFIG, config], {
isMergeableObject: isPlainObject,
});
return [null, normalizeConfig(mergedConfig)];
}

Expand Down Expand Up @@ -959,7 +962,9 @@ export function loadAndValidateConfig(flags: CLIFlags, pkgManifest: any): Snowpa
{webDependencies: pkgManifest.webDependencies},
config,
cliConfig as any,
]);
], {
isMergeableObject: isPlainObject,
});
for (const webDependencyName of Object.keys(mergedConfig.webDependencies || {})) {
if (pkgManifest.dependencies && pkgManifest.dependencies[webDependencyName]) {
handleConfigError(
Expand Down
12 changes: 12 additions & 0 deletions test/build/config-instantiated-object/__snapshots__
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`snowpack build config-instantiated-object: allFiles 1`] = `
Array [
"__snowpack__/env.js",
"_dist_/index.js",
]
`;

exports[`snowpack build config-instantiated-object: build/__snowpack__/env.js 1`] = `"export default {\\"MODE\\":\\"production\\",\\"NODE_ENV\\":\\"production\\",\\"SSR\\":false};"`;

exports[`snowpack build config-instantiated-object: build/_dist_/index.js 1`] = `"console.log('fooey');"`;
11 changes: 11 additions & 0 deletions test/build/config-instantiated-object/dummy-plugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module.exports = (snowpackConfig, { instance }) => {
if(!instance.prop) {
throw new Error("simple prop value didn't make it");
}

if(!instance.method) {
throw new Error("method value didn't make it");
}

return { name : "dummy-plugin" };
};
13 changes: 13 additions & 0 deletions test/build/config-instantiated-object/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"private": true,
"version": "1.0.1",
"name": "@snowpack/test-config-instantiated-object",
"description": "Test for plugin options using instantiated objects",
"scripts": {
"testbuild": "snowpack build"
},
"devDependencies": {
"cross-env": "^7.0.2",
"snowpack": "^2.14.3"
}
}
18 changes: 18 additions & 0 deletions test/build/config-instantiated-object/snowpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class Instantiable {
constructor() {
this.prop = true;
}

method() {}
}

const instance = new Instantiable();

module.exports = {
mount: {
'./src': '/_dist_',
},
plugins: [
[ "./dummy-plugin.js", { instance } ]
]
};
1 change: 1 addition & 0 deletions test/build/config-instantiated-object/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('fooey');
12 changes: 12 additions & 0 deletions test/build/config-js-file/__snapshots__
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`snowpack build config-js-file: allFiles 1`] = `
Array [
"__snowpack__/env.js",
"_dist_/index.js",
]
`;

exports[`snowpack build config-js-file: build/__snowpack__/env.js 1`] = `"export default {\\"MODE\\":\\"production\\",\\"NODE_ENV\\":\\"production\\",\\"SSR\\":false};"`;

exports[`snowpack build config-js-file: build/_dist_/index.js 1`] = `"console.log('fooey');"`;
13 changes: 13 additions & 0 deletions test/build/config-js-file/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"private": true,
"version": "1.0.1",
"name": "@snowpack/test-config-js-file",
"description": "Test for a js config file",
"scripts": {
"testbuild": "snowpack build"
},
"devDependencies": {
"cross-env": "^7.0.2",
"snowpack": "^2.14.3"
}
}
5 changes: 5 additions & 0 deletions test/build/config-js-file/snowpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
mount: {
'./src': '/_dist_',
},
};
1 change: 1 addition & 0 deletions test/build/config-js-file/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('fooey');

1 comment on commit 0b3813f

@vercel
Copy link

@vercel vercel bot commented on 0b3813f Nov 8, 2020

Choose a reason for hiding this comment

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

Please sign in to comment.