Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Commit

Permalink
fix: Issue with npm module dependencies being pulled into shared refe…
Browse files Browse the repository at this point in the history
…rence (#955)

closes #955
  • Loading branch information
nchanged committed Nov 27, 2017
1 parent 39fc7f8 commit 1aa514c
Show file tree
Hide file tree
Showing 22 changed files with 200 additions and 20 deletions.
1 change: 1 addition & 0 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ gulp.task("installDevDeps", function(done) {
"vue-template-compiler",
"vue-template-es2015-compiler",
"vue",
"jwt-decode",
"vue-server-renderer",
"vue-hot-reload-api",
"vue-class-component",
Expand Down
56 changes: 39 additions & 17 deletions src/quantum/plugin/QuantumBit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ export class QuantumBit {
if (origin === false && !dependent.quantumBit) {
pkg.quantumBitBanned = true;
}
})


});
})
}
return true;
Expand All @@ -102,6 +100,18 @@ export class QuantumBit {
}
}

private findRootDependents(f: FileAbstraction, list: FileAbstraction[]): FileAbstraction[] {
if (list.indexOf(f) === -1) {
list.push(f);
if (f !== this.entry) {
f.dependents.forEach(dep => {
this.findRootDependents(dep, list);
});
}
}
return list;
}

public resolve(file?: FileAbstraction) {
if (this.isEntryModule) {
this.dealWithModule(this.entry, true);
Expand All @@ -110,30 +120,26 @@ export class QuantumBit {
}

this.populateDependencies(this.entry);
const findRootDependents = (f: FileAbstraction, list: FileAbstraction[]): FileAbstraction[] => {
if (list.indexOf(f) === -1) {
list.push(f);
if (f !== this.entry) {
f.dependents.forEach(dep => {
findRootDependents(dep, list);
});
}
}
return list;
}


for (const p of this.candidates) {
const file = p[1];
const rootDependents = findRootDependents(file, []);
const rootDependents = this.findRootDependents(file, []);
for (const root of rootDependents) {
if (!root.quantumBit && root !== this.entry) { file.quantumBitBanned = true; }
if (!root.quantumBit && root !== this.entry) {
file.quantumBitBanned = true;
}
else {
if (root.quantumBit && root.quantumBit !== this && root !== this.entry) {
file.quantumBitBanned = true;
}
}
}
if (!file.quantumBit) { file.quantumBitBanned = true; };
if (!file.quantumBit) {
file.quantumBitBanned = true;
};
}

for (const item of this.modulesCanidates) {
const moduleCandidate = item[1];
// a case where the same library is imported dynamically and through require statements
Expand Down Expand Up @@ -165,6 +171,22 @@ export class QuantumBit {
})
}
}

this.modulesCanidates.forEach(pkg => {
if (!pkg.quantumBitBanned) {
pkg.fileAbstractions.forEach(f => {
const dependents = this.findRootDependents(f, []);
dependents.forEach(dep => {
if (!dep.quantumBit && dep !== this.entry) { pkg.quantumBitBanned = true; }
else {
if (dep.quantumBit && dep.quantumBit !== this && dep !== this.entry) {
pkg.quantumBitBanned = true;
}
}
});
});
}
})
}

public populate() {
Expand Down
63 changes: 60 additions & 3 deletions src/tests/dynamic_imports_test/CodeSplittingFileIntegrity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,6 @@ export class CodeSplittingFileIntegrityTest {
project: {
files: {
"index.ts": `
export async function test() {
await import("./components/HomeComponent")
const lib_b_2 = await import('lib_b');
Expand Down Expand Up @@ -711,7 +710,7 @@ export class CodeSplittingFileIntegrityTest {
"Should ignore a file with nested references"() {
return FuseTestEnv.create(
{
testFolder: "_current_test",
// testFolder: "_current_test",
project: {
fromStubs: "quantum_split_complicated",
plugins: [
Expand Down Expand Up @@ -739,7 +738,7 @@ export class CodeSplittingFileIntegrityTest {
'// default/common/ui/layout/header/header.js',
'// default/common/ui/layout/content/index.js',
'// default/common/ui/layout/content/content.js',
'// default/modules/test/routes/another-test-route.js'
//'// default/modules/test/routes/another-test-route.js'
]
sharedFiles.forEach(file => {
master.shouldFindString(file);
Expand All @@ -759,6 +758,64 @@ export class CodeSplittingFileIntegrityTest {
split2.shouldFindString('// default/modules/test/views/another-test-component/bar.js')


split3.shouldFindString('// default/modules/test/views/test-component-header/index.js')
split3.shouldFindString('// default/modules/test/views/test-component-header/test-component-header.jsx')
}));
}

"Should not split a library"() {
return FuseTestEnv.create(
{
// testFolder: "_current_test",
project: {
fromStubs: "quantum_split_2",
plugins: [
QuantumPlugin({
target: "browser"
})
]
}
}
)
.simple().then(test => test.browser((window, env) => {
window.$fsx.r(0);
const master = env.getScript("app.js");
const split1 = env.getScript("5b053b5d.js");
const split2 = env.getScript("621a109b.js");
const split3 = env.getScript("fa62310f.js");

const sharedFiles = [
'// default/index.js',
'// default/route-loader.js',
'// default/modules/test/routes/index.js',
'// default/modules/test/routes/test-route.js',
`// default/common/routes/index.js`,
`// default/common/routes/links.js`,
`// default/common/routes/route.js`,
`// default/common/store/index.js`,
`// default/common/store/store.js`,
`// default/common/auth/index.js`,
`// default/common/auth/token.js`,
`// default/modules/test/routes/another-test-route.js`,
`// jwt-decode/lib/index.js`,
`// jwt-decode/lib/base64_url_decode.js`,
`// jwt-decode/lib/atob.js`
]
sharedFiles.forEach(file => {
master.shouldFindString(file);
split1.shouldNotFindString(file)
split2.shouldNotFindString(file)
split3.shouldNotFindString(file)
});


split1.shouldFindString('// default/modules/test/views/test-component/index.js')
split1.shouldFindString('// default/modules/test/views/test-component/test-component.jsx')


split2.shouldFindString('// default/modules/test/views/another-test-component/index.js')
split2.shouldFindString('// default/modules/test/views/another-test-component/another-test-component.jsx')

split3.shouldFindString('// default/modules/test/views/test-component-header/index.js')
split3.shouldFindString('// default/modules/test/views/test-component-header/test-component-header.jsx')
}));
Expand Down
1 change: 1 addition & 0 deletions src/tests/stubs/cases/quantum_split_2/common/auth/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./token";
5 changes: 5 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/common/auth/token.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import * as decode from "jwt-decode";
export function jwtdecode() {
console.log(decode);
return decode("");
}
2 changes: 2 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/common/routes/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./links";
export * from "./route";
4 changes: 4 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/common/routes/links.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const links = {
test: "/test",
whatever: "/whatever"
};
5 changes: 5 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/common/routes/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Store } from "../store";
export const route = () => {
console.log("i am a routing object");
console.log("setting up store", Store);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./store";
7 changes: 7 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/common/store/store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { jwtdecode } from "../auth";

console.log("here shit");
export const Store = () => {
console.log('********************************');
console.log("decode?", jwtdecode);
};
3 changes: 3 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { loadRoute } from "./route-loader";

loadRoute();
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const anotherTestRoute = {
component: () => import("../views/another-test-component"),
moreComponents: [() => import("../views/test-component-header")]
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./test-route";
export * from "./another-test-route";
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { route } from "../../../common/routes";

export const testRoute = {
component: () => import("../views/test-component"),
moreComponents: [() => import("../views/test-component-header")]
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { links } from "../../../../common/routes";
import { jwtdecode } from "../../../../common/auth";

export function AnotherTestComponent() {
console.log("I am another test component");
console.log("I also reference links", links);
console.log("and I reference the auth function", jwtdecode());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { AnotherTestComponent } from "./another-test-component";
export = AnotherTestComponent;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { TestComponentHeader } from "./test-component-header";
export = TestComponentHeader;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function TestComponentHeader() {
console.log("I am a test component header");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { TestComponent } from "./test-component";
export = TestComponent;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { links } from "../../../../common/routes";

// this bundle is receiving ALL of the common/ui/layout module split into it,
// even though this isn't the only bundle using it.
export function TestComponent() {
console.log("I am a test component");
console.log("here are my links", links);
}
25 changes: 25 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/route-loader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { testRoute, anotherTestRoute } from "./modules/test/routes";

import { route } from "./common/routes";

// this function is referenced in our split bundles too.
// for some reason, the first split bundle is receiving ALL of
// the common bundle files

// this would get loaded into the router
const routeLoader = [anotherTestRoute, testRoute];

// mock router call
export async function loadRoute() {
routeLoader.forEach(r => {
runRoute(r);
});
}

async function runRoute(r: typeof testRoute) {
const component = await r.component();
component();

const components = await Promise.all(r.moreComponents.map(r => r()));
components.forEach(c => c());
}
10 changes: 10 additions & 0 deletions src/tests/stubs/cases/quantum_split_2/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "ES6",
"jsx": "react",
"importHelpers": true,
"emitDecoratorMetadata": true,
"experimentalDecorators": true
}
}

0 comments on commit 1aa514c

Please sign in to comment.