From 51e2611ff40cefbb1ad3450948c77df82720fa6d Mon Sep 17 00:00:00 2001 From: filipenevola Date: Tue, 23 Jun 2020 14:12:18 -0400 Subject: [PATCH 01/35] WIP dead code elimination --- .../.npm/package/npm-shrinkwrap.json | 12 ++-- packages/minifier-js/minifier.js | 63 ++++++++++++++++--- packages/minifier-js/package.js | 2 +- .../standard-minifier-js/plugin/minify-js.js | 35 ++++++++--- tools/isobuild/bundler.js | 4 +- 5 files changed, 88 insertions(+), 28 deletions(-) diff --git a/packages/minifier-js/.npm/package/npm-shrinkwrap.json b/packages/minifier-js/.npm/package/npm-shrinkwrap.json index fae47c7a372..712e9dd22d5 100644 --- a/packages/minifier-js/.npm/package/npm-shrinkwrap.json +++ b/packages/minifier-js/.npm/package/npm-shrinkwrap.json @@ -17,14 +17,14 @@ "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==" }, "source-map-support": { - "version": "0.5.16", - "resolved": "https://registry.npmjs.org/source-map-support/-/source-map-support-0.5.16.tgz", - "integrity": "sha512-efyLRJDr68D9hBBNIPWFjhpFzURh+KJykQwvMyW5UiZzYwoF6l4YMMDIJJEyFWxWCqfyxLzz6tSfUFR+kXXsVQ==" + "version": "0.5.19", + "resolved": "https://registry.npmjs.org/source-map-support/-/source-map-support-0.5.19.tgz", + "integrity": "sha512-Wonm7zOCIJzBGQdB+thsPar0kYuCIzYvxZwlBa87yi/Mdjv7Tip2cyVbLj5o0cFPN4EVkuTwb3GDDyUx2DGnGw==" }, "terser": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/terser/-/terser-4.4.0.tgz", - "integrity": "sha512-oDG16n2WKm27JO8h4y/w3iqBGAOSCtq7k8dRmrn4Wf9NouL0b2WpMHGChFGZq4nFAQy1FsNJrVQHfurXOSTmOA==" + "version": "4.7.0", + "resolved": "https://registry.npmjs.org/terser/-/terser-4.7.0.tgz", + "integrity": "sha512-Lfb0RiZcjRDXCC3OSHJpEkxJ9Qeqs6mp2v4jf2MHfy8vGERmVDuvjXdd/EnP5Deme5F2yBRBymKmKHCBg2echw==" } } } diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index 87d74c16879..71d4744b53e 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -1,35 +1,81 @@ var terser; -meteorJsMinify = function (source) { +const getGlobalDefsOptions = ({ arch }) => ({ + "Meteor.isServer": false, + "Meteor.isTest": false, + "Meteor.isDevelopment": false, + "Meteor.isClient": true, + "Meteor.isProduction": true, + "Meteor.isCordova": arch === 'web.cordova', +}); + +meteorJsMinify = function (source, options) { var result = {}; var NODE_ENV = process.env.NODE_ENV || "development"; - terser = terser || Npm.require("terser"); + const globalDefs = getGlobalDefsOptions(options); + const globalDefsMapping = Object.entries(globalDefs).reduce((acc, [from, to]) => { + const parts = from.split('.'); + if (parts.length < 2) { + return acc; + } + const startValue = parts[0]; + const endValue = parts[1]; + return ({ + ...acc, + [startValue]: { + ...acc[startValue], [endValue]: to + } + }); + }, {}); try { - var terserResult = terser.minify(source, { + var ast = terser.minify(source, { + compress: false, + mangle: false, + output: { + ast: true, + code: false + }, + safari10: true, + }).ast + const transformer = new terser.TreeTransformer(node => { + if (node instanceof terser.AST_Dot) { + const globalDefsForStart = globalDefsMapping[node.start.value]; + const mappingForEnds = globalDefsForStart && globalDefsForStart[node.end.value]; + + if (mappingForEnds != null) { + console.log(`node`, node); + + return mappingForEnds ? new terser.AST_True(node) : new terser.AST_False(node); + } + } + }) + var optimizedAst = ast.transform(transformer) + var terserResult = terser.minify(optimizedAst, { compress: { drop_debugger: false, unused: false, dead_code: true, global_defs: { - "process.env.NODE_ENV": NODE_ENV - } + "process.env.NODE_ENV": NODE_ENV, + "process.env.NODE_DEBUG": false, + }, + // passes: 2 }, + // mangle: {toplevel: true}, // Fix issue #9866, as explained in this comment: // https://github.com/mishoo/UglifyJS2/issues/1753#issuecomment-324814782 // And fix terser issue #117: https://github.com/terser-js/terser/issues/117 safari10: true, }); - if (typeof terserResult.code === "string") { result.code = terserResult.code; result.minifier = 'terser'; } else { throw terserResult.error || - new Error("unknown terser.minify failure"); + new Error("unknown terser.minify failure"); } - } catch (e) { // Although Babel.minify can handle a wider variety of ECMAScript // 2015+ syntax, it is substantially slower than UglifyJS/terser, so @@ -40,6 +86,5 @@ meteorJsMinify = function (source) { result.code = Babel.minify(source, options).code; result.minifier = 'babel-minify'; } - return result; }; diff --git a/packages/minifier-js/package.js b/packages/minifier-js/package.js index ddb435e8cf0..b33b80d0aba 100644 --- a/packages/minifier-js/package.js +++ b/packages/minifier-js/package.js @@ -4,7 +4,7 @@ Package.describe({ }); Npm.depends({ - terser: "4.4.0" + terser: "4.7.0" }); Package.onUse(function (api) { diff --git a/packages/standard-minifier-js/plugin/minify-js.js b/packages/standard-minifier-js/plugin/minify-js.js index d1d67482d1b..3bf2f8011c3 100644 --- a/packages/standard-minifier-js/plugin/minify-js.js +++ b/packages/standard-minifier-js/plugin/minify-js.js @@ -1,4 +1,4 @@ -import { extractModuleSizesTree } from "./stats.js"; +import {extractModuleSizesTree} from "./stats.js"; Plugin.registerMinifier({ extensions: ['js'], @@ -8,10 +8,13 @@ Plugin.registerMinifier({ return minifier; }); -function MeteorBabelMinifier () {}; +function MeteorBabelMinifier() { +}; -MeteorBabelMinifier.prototype.processFilesForBundle = function(files, options) { +MeteorBabelMinifier.prototype.processFilesForBundle = function (files, options) { var mode = options.minifyMode; + console.log(`filipe:options`, JSON.stringify(options)); + console.log(`filipe:mode`, mode); // don't minify anything for development if (mode === 'development') { @@ -114,36 +117,48 @@ MeteorBabelMinifier.prototype.processFilesForBundle = function(files, options) { data: "", stats: Object.create(null) }; + const FILES_TO_LOG = []; + console.log(`filipe:FILES_TO_LOG`, FILES_TO_LOG); files.forEach(file => { // Don't reminify *.min.js. - if (/\.min\.js$/.test(file.getPathInBundle())) { - toBeAdded.data += file.getContentsAsString(); + const content = file.getContentsAsString(); + const filePath = file.getPathInBundle(); + if (/\.min\.js$/.test(filePath)) { + toBeAdded.data += content; } else { var minified; try { - minified = meteorJsMinify(file.getContentsAsString()); + console.log(`filipe:file.getPathInBundle()`, filePath); + if (FILES_TO_LOG.some(fileName => filePath.includes(fileName))) { + console.log('filipe:ORIGINAL', filePath); + console.log(content); + } + + minified = meteorJsMinify(content, options); if (!(minified && typeof minified.code === "string")) { throw new Error(); } } catch (err) { - var filePath = file.getPathInBundle(); - maybeThrowMinifyErrorBySourceFile(err, file); err.message += " while minifying " + filePath; throw err; } + if (FILES_TO_LOG.some(fileName => filePath.includes(fileName))) { + console.log('filipe:MINIFIED', filePath); + console.log(minified.code); + } const tree = extractModuleSizesTree(minified.code); if (tree) { - toBeAdded.stats[file.getPathInBundle()] = + toBeAdded.stats[filePath] = [Buffer.byteLength(minified.code), tree]; } else { - toBeAdded.stats[file.getPathInBundle()] = + toBeAdded.stats[filePath] = Buffer.byteLength(minified.code); } diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 5311ebbfd63..ea7a95b2533 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1368,9 +1368,9 @@ class Target { buildmessage.enterJob('minifying app code', function () { try { Promise.all([ - markedMinifier(staticFiles, { minifyMode }), + markedMinifier(staticFiles, { minifyMode, arch }), ...dynamicFiles.map( - file => markedMinifier([file], { minifyMode }) + file => markedMinifier([file], { minifyMode, arch }) ), ]).await(); } catch (e) { From d717389540eeb602c8bf4cf72985408647c14533 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Mon, 17 Aug 2020 17:03:16 -0300 Subject: [PATCH 02/35] Dead code elimination based on meteor build info, use babel instead of terser for replacing nodes on AST. Also exclude case of assignment. --- packages/babel-compiler/babel.js | 32 ++++++++++++++++++++++++++++++++ packages/minifier-js/minifier.js | 25 ++----------------------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/packages/babel-compiler/babel.js b/packages/babel-compiler/babel.js index 2adbbae9252..0f555928886 100644 --- a/packages/babel-compiler/babel.js +++ b/packages/babel-compiler/babel.js @@ -47,5 +47,37 @@ Babel = { getMinimumModernBrowserVersions: function () { return Npm.require("meteor-babel/modern-versions.js").get(); + }, + replaceMeteorInternalState: function(source, globalDefsMapping) { + try { + const globalDefsKeys = Object.keys(globalDefsMapping); + const replacedCode = Npm.require("@babel/core").transformSync(source, { + plugins: [ + function replaceStateVars({types: t}) { + return { + visitor: { + MemberExpression(path) { + const object = path.node.object.name; + const property = path.node.property.name; + const globalDefsForStart = object && globalDefsKeys.indexOf(object) > -1 && globalDefsMapping[object]; + const mappingForEnds = property && globalDefsForStart + && Object.keys(globalDefsForStart).indexOf(property) > -1 + ? globalDefsForStart[property] : null; + + if (mappingForEnds !== null && path.parentPath.node.type !== "AssignmentExpression") { + path.replaceWith( + t.booleanLiteral(!!mappingForEnds) + ); + } + }, + }, + }; + }, + ], + }).code; + return replacedCode; + } catch(){ + return source; + } } }; diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index 71d4744b53e..eb337dba744 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -30,29 +30,8 @@ meteorJsMinify = function (source, options) { }); }, {}); try { - var ast = terser.minify(source, { - compress: false, - mangle: false, - output: { - ast: true, - code: false - }, - safari10: true, - }).ast - const transformer = new terser.TreeTransformer(node => { - if (node instanceof terser.AST_Dot) { - const globalDefsForStart = globalDefsMapping[node.start.value]; - const mappingForEnds = globalDefsForStart && globalDefsForStart[node.end.value]; - - if (mappingForEnds != null) { - console.log(`node`, node); - - return mappingForEnds ? new terser.AST_True(node) : new terser.AST_False(node); - } - } - }) - var optimizedAst = ast.transform(transformer) - var terserResult = terser.minify(optimizedAst, { + var optimizedCode = Babel.replaceMeteorInternalState(source, globalDefsMapping) + var terserResult = terser.minify(optimizedCode, { compress: { drop_debugger: false, unused: false, From be4cfbfc490a8c9141d2d559f0c05cebe066b388 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Mon, 17 Aug 2020 19:45:16 -0300 Subject: [PATCH 03/35] Fix build error --- packages/babel-compiler/babel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-compiler/babel.js b/packages/babel-compiler/babel.js index 0f555928886..7a4e8d8c33a 100644 --- a/packages/babel-compiler/babel.js +++ b/packages/babel-compiler/babel.js @@ -76,7 +76,7 @@ Babel = { ], }).code; return replacedCode; - } catch(){ + } catch(e){ return source; } } From c310d9a4be56c6bf2ea0324dc5ff6421b9603b7e Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Mon, 24 Aug 2020 17:54:23 -0300 Subject: [PATCH 04/35] Support for custom global flags when minifying --- packages/babel-compiler/babel.js | 34 +++++++++++-------- packages/minifier-js/minifier.js | 24 +++++++++++-- .../standard-minifier-js/plugin/minify-js.js | 13 ------- 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/packages/babel-compiler/babel.js b/packages/babel-compiler/babel.js index 7a4e8d8c33a..44e5d44337f 100644 --- a/packages/babel-compiler/babel.js +++ b/packages/babel-compiler/babel.js @@ -51,31 +51,35 @@ Babel = { replaceMeteorInternalState: function(source, globalDefsMapping) { try { const globalDefsKeys = Object.keys(globalDefsMapping); - const replacedCode = Npm.require("@babel/core").transformSync(source, { + return Npm.require("@babel/core").transformSync(source, { + ast: false, + compact: false, plugins: [ function replaceStateVars({types: t}) { return { visitor: { - MemberExpression(path) { - const object = path.node.object.name; - const property = path.node.property.name; - const globalDefsForStart = object && globalDefsKeys.indexOf(object) > -1 && globalDefsMapping[object]; - const mappingForEnds = property && globalDefsForStart - && Object.keys(globalDefsForStart).indexOf(property) > -1 - ? globalDefsForStart[property] : null; + MemberExpression: { + exit(path) { + const object = path.node.object.name; + const property = path.node.property.name; + const globalDefsForStart = object && globalDefsKeys.indexOf(object) > -1 && globalDefsMapping[object]; + const mappingForEnds = property && globalDefsForStart + && Object.keys(globalDefsForStart).indexOf(property) > -1 + ? globalDefsForStart[property] : null; - if (mappingForEnds !== null && path.parentPath.node.type !== "AssignmentExpression") { - path.replaceWith( - t.booleanLiteral(!!mappingForEnds) - ); - } - }, + if (mappingForEnds !== null && path.parentPath.node.type !== "AssignmentExpression") { + path.replaceWith( + t.booleanLiteral(mappingForEnds === 'true' || mappingForEnds === true) + ); + path.skip(); + } + }, + } }, }; }, ], }).code; - return replacedCode; } catch(e){ return source; } diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index eb337dba744..cb880d0ee78 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -15,7 +15,26 @@ meteorJsMinify = function (source, options) { terser = terser || Npm.require("terser"); const globalDefs = getGlobalDefsOptions(options); - const globalDefsMapping = Object.entries(globalDefs).reduce((acc, [from, to]) => { + let customSettings = {}; + + /* + MINIFIER_SETTINGS_FILE=settings.json + settings.json: + { + globalDefinitions: { + isAdmin: true + } + } + will replace Meteor.isAdmin with true + */ + if(process.env.MINIFIER_SETTINGS_FILE) { + const settings = Npm.require("fs").readFileSync(process.env.MINIFIER_SETTINGS_FILE); + const minifierSettings = JSON.parse(settings.toString()); + customSettings = minifierSettings && minifierSettings.globalDefinitions || {}; + } + + + var globalDefsMapping = Object.entries(globalDefs).reduce((acc, [from, to]) => { const parts = from.split('.'); if (parts.length < 2) { return acc; @@ -29,12 +48,13 @@ meteorJsMinify = function (source, options) { } }); }, {}); + globalDefsMapping.Meteor = {...globalDefsMapping.Meteor, ...customSettings}; try { var optimizedCode = Babel.replaceMeteorInternalState(source, globalDefsMapping) var terserResult = terser.minify(optimizedCode, { compress: { drop_debugger: false, - unused: false, + unused: true, dead_code: true, global_defs: { "process.env.NODE_ENV": NODE_ENV, diff --git a/packages/standard-minifier-js/plugin/minify-js.js b/packages/standard-minifier-js/plugin/minify-js.js index 3bf2f8011c3..e668526518f 100644 --- a/packages/standard-minifier-js/plugin/minify-js.js +++ b/packages/standard-minifier-js/plugin/minify-js.js @@ -13,8 +13,6 @@ function MeteorBabelMinifier() { MeteorBabelMinifier.prototype.processFilesForBundle = function (files, options) { var mode = options.minifyMode; - console.log(`filipe:options`, JSON.stringify(options)); - console.log(`filipe:mode`, mode); // don't minify anything for development if (mode === 'development') { @@ -117,8 +115,6 @@ MeteorBabelMinifier.prototype.processFilesForBundle = function (files, options) data: "", stats: Object.create(null) }; - const FILES_TO_LOG = []; - console.log(`filipe:FILES_TO_LOG`, FILES_TO_LOG); files.forEach(file => { // Don't reminify *.min.js. @@ -130,11 +126,6 @@ MeteorBabelMinifier.prototype.processFilesForBundle = function (files, options) var minified; try { - console.log(`filipe:file.getPathInBundle()`, filePath); - if (FILES_TO_LOG.some(fileName => filePath.includes(fileName))) { - console.log('filipe:ORIGINAL', filePath); - console.log(content); - } minified = meteorJsMinify(content, options); @@ -149,10 +140,6 @@ MeteorBabelMinifier.prototype.processFilesForBundle = function (files, options) throw err; } - if (FILES_TO_LOG.some(fileName => filePath.includes(fileName))) { - console.log('filipe:MINIFIED', filePath); - console.log(minified.code); - } const tree = extractModuleSizesTree(minified.code); if (tree) { toBeAdded.stats[filePath] = From 11fe73fff4d946031654ef2333b18ee34ac9e8b0 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 25 Aug 2020 16:12:55 -0300 Subject: [PATCH 05/35] Support for custom global flags when running --- packages/meteor/client_environment.js | 3 ++- packages/meteor/server_environment.js | 4 ++++ packages/minifier-js/minifier.js | 8 +++----- tools/cli/commands.js | 10 +++++----- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/meteor/client_environment.js b/packages/meteor/client_environment.js index 074c655bc1d..5bbeb9ef40b 100644 --- a/packages/meteor/client_environment.js +++ b/packages/meteor/client_environment.js @@ -53,7 +53,8 @@ Meteor = { * @static * @type {Boolean} */ - isModern: config.isModern + isModern: config.isModern, + ...config.GLOBAL_DEFINITIONS }; if (config.gitCommitHash) { diff --git a/packages/meteor/server_environment.js b/packages/meteor/server_environment.js index fa212ca30cc..84f85878364 100644 --- a/packages/meteor/server_environment.js +++ b/packages/meteor/server_environment.js @@ -26,6 +26,9 @@ Meteor.settings = {}; if (process.env.METEOR_SETTINGS) { try { Meteor.settings = JSON.parse(process.env.METEOR_SETTINGS); + if(Meteor.settings.globalDefinitions){ + Object.assign(Meteor, Meteor.settings.globalDefinitions); + } } catch (e) { throw new Error("METEOR_SETTINGS are not valid JSON."); } @@ -44,6 +47,7 @@ if (! Meteor.settings.public) { // settings will be sent to the client. if (config) { config.PUBLIC_SETTINGS = Meteor.settings.public; + config.GLOBAL_DEFINITIONS = Meteor.settings.globalDefinitions ? Meteor.settings.globalDefinitions : {}; } if (config && config.gitCommitHash) { diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index cb880d0ee78..39597eb7128 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -18,7 +18,6 @@ meteorJsMinify = function (source, options) { let customSettings = {}; /* - MINIFIER_SETTINGS_FILE=settings.json settings.json: { globalDefinitions: { @@ -27,10 +26,9 @@ meteorJsMinify = function (source, options) { } will replace Meteor.isAdmin with true */ - if(process.env.MINIFIER_SETTINGS_FILE) { - const settings = Npm.require("fs").readFileSync(process.env.MINIFIER_SETTINGS_FILE); - const minifierSettings = JSON.parse(settings.toString()); - customSettings = minifierSettings && minifierSettings.globalDefinitions || {}; + if(process.env.METEOR_SETTINGS) { + const settings = JSON.parse(process.env.METEOR_SETTINGS); + customSettings = settings && settings.globalDefinitions || {}; } diff --git a/tools/cli/commands.js b/tools/cli/commands.js index da9adc786dc..b732d41e0f2 100644 --- a/tools/cli/commands.js +++ b/tools/cli/commands.js @@ -927,6 +927,7 @@ var buildCommands = { architecture: { type: String }, "server-only": { type: Boolean }, 'mobile-settings': { type: String }, + 'settings': { type: String }, server: { type: String }, "cordova-server-port": { type: String }, // XXX COMPAT WITH 0.9.2.2 @@ -1017,10 +1018,9 @@ var buildCommand = function (options) { const serverOnly = options._bundleOnly || !!options['server-only']; // options['mobile-settings'] is used to set the initial value of - // `Meteor.settings` on mobile apps. Pass it on to options.settings, - // which is used in this command. - if (options['mobile-settings']) { - options.settings = options['mobile-settings']; + // `Meteor.settings` on mobile apps. + if (options.settings) { + process.env.METEOR_SETTINGS = files.readFile(options.settings, 'utf8'); } const appName = files.pathBasename(options.appDir); @@ -1138,7 +1138,7 @@ ${Console.command("meteor build ../output")}`, import { CordovaProject } from '../cordova/project.js'; cordovaProject = new CordovaProject(projectContext, { - settingsFile: options.settings, + settingsFile: options['mobile-settings'], mobileServerUrl: utils.formatUrl(parsedMobileServerUrl), cordovaServerPort: parsedCordovaServerPort }); if (buildmessage.jobHasMessages()) return; From c65bd60821c4a65ce6173e765d42b22867eb9d5b Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Fri, 4 Sep 2020 13:30:33 -0300 Subject: [PATCH 06/35] Prepare a map of imports to be used inside the bundler for removing unused exports --- tools/isobuild/bundler.js | 16 ++++++++++++++++ tools/isobuild/import-scanner.ts | 24 ++++++++++++++++++------ tools/isobuild/js-analyze.js | 25 +++++++++++++++++++++---- 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 7e4cec22070..9fdeaf22990 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1162,6 +1162,22 @@ class Target { const versions = {}; const dynamicImportFiles = new Set; + // importMap has all the import { x } from '...' + // info from every target/bundle in the build system + const importMap = new Map; + sourceBatches.forEach((sourceBatch) => { + const unibuild = sourceBatch.unibuild; + const name = unibuild.pkg.name || null; + jsOutputFilesMap.get(name).files.forEach((file) => { + file.imports.forEach(([key,object]) => { + importMap.set(key, [...importMap.get(key), ...object]) + }) + }) + }) + + // now we need to remove the exports, and let the minifier do it's job later + + // Copy their resources into the bundle in order sourceBatches.forEach((sourceBatch) => { const unibuild = sourceBatch.unibuild; diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index e6c09760e2d..3787bdf0dca 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -340,6 +340,7 @@ interface File extends RawFile { servePath?: string; absModuleId?: string; deps?: Record; + imports?: Record; lazy: boolean; bare?: boolean; // TODO Improve the sourceMap type. @@ -369,6 +370,10 @@ type ImportInfo = { parentWasDynamic?: boolean; helpers?: Record; } +type JSAnalyzeInfo = { + dependencies?: Record; + identifiers?: Record; +} export default class ImportScanner { public name: string; @@ -927,12 +932,13 @@ export default class ImportScanner { // Return all installable output files that are either eager or // imported (statically or dynamically). - return this.outputFiles.filter(file => { + const files = this.outputFiles.filter(file => { return file.absModuleId && - ! file[fakeSymbol] && - ! file.hasErrors && - (! file.lazy || file.imported); + ! file[fakeSymbol] && + ! file.hasErrors && + (! file.lazy || file.imported); }); + return files; } private getSourcePath(file: File) { @@ -975,7 +981,7 @@ export default class ImportScanner { private findImportedModuleIdentifiers( file: File, - ): Record { + ): JSAnalyzeInfo { if (IMPORT_SCANNER_CACHE.has(file.hash)) { return IMPORT_SCANNER_CACHE.get(file.hash); } @@ -1089,7 +1095,12 @@ export default class ImportScanner { } try { - file.deps = file.deps || this.findImportedModuleIdentifiers(file); + let importInfo; + if(!file.deps){ + importInfo = this.findImportedModuleIdentifiers(file); + } + file.deps = file.deps || importInfo?.identifiers; + file.imports = file.imports || importInfo?.dependencies; } catch (e) { if (e.$ParseError) { (buildmessage as any).error(e.message, { @@ -1118,6 +1129,7 @@ export default class ImportScanner { } let depFile = this.getFile(absImportedPath); + if (depFile) { // We should never have stored a fake file in this.outputFiles, so // it's surprising if depFile[fakeSymbol] is true. diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index 04cac3fd5ab..edf8eeba2a9 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -72,13 +72,14 @@ export function findImportedModuleIdentifiers(source, hash) { const ast = tryToParse(source, hash); importedIdentifierVisitor.visit(ast, source, possibleIndexes); - return importedIdentifierVisitor.identifiers; + return { identifiers:importedIdentifierVisitor.identifiers, dependencies:importedIdentifierVisitor.dependencies }; } const importedIdentifierVisitor = new (class extends Visitor { reset(rootPath, code, possibleIndexes) { this.requireIsBound = false; this.identifiers = Object.create(null); + this.dependencies = Object.create(null); // Defining this.possibleIndexes causes the Visitor to ignore any // subtrees of the AST that do not contain any indexes of identifiers @@ -112,6 +113,18 @@ const importedIdentifierVisitor = new (class extends Visitor { } } + addDependency(id, importIdentifiers, sideEffects) { + this.dependencies[id] = this.dependencies[id] ? + { + deps: new Set([...this.dependencies[id].deps, ...importIdentifiers]), + sideEffects: this.dependencies[id].sideEffects || sideEffects, + } : + { + deps: importIdentifiers, + sideEffects + }; + } + visitFunctionExpression(path) { return this._functionParamRequireHelper(path); } @@ -139,7 +152,6 @@ const importedIdentifierVisitor = new (class extends Visitor { visitCallExpression(path) { const node = path.getValue(); const args = node.arguments; - const argc = args.length; const firstArg = args[0]; this.visitChildren(path); @@ -150,11 +162,9 @@ const importedIdentifierVisitor = new (class extends Visitor { if (isIdWithName(node.callee, "require")) { this.addIdentifier(firstArg.value, "require"); - } else if (node.callee.type === "Import" || isIdWithName(node.callee, "import")) { this.addIdentifier(firstArg.value, "import", true); - } else if (node.callee.type === "MemberExpression" && // The Reify compiler sometimes renames references to the // CommonJS module object for hygienic purposes, but it @@ -165,6 +175,13 @@ const importedIdentifierVisitor = new (class extends Visitor { isPropertyWithName(node.callee.property, "dynamicImport"); if (propertyName) { + // if we have an object definition on module.link(), we are importing with ES6 possible without side effects + // otherwise we are considering it as a side effect import + if(args[1]) { + this.addDependency(firstArg.value, args[1].properties.map(({key}) => key.name), false); + }else{ + this.addDependency(firstArg.value, [], true); + } this.addIdentifier( firstArg.value, "import", From d7e2ac85fd30385f9fb2f382b25f02773559ea7a Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 8 Sep 2020 18:03:33 -0300 Subject: [PATCH 07/35] Starting to remove some exports from the final bundle, there is still work to do regarding global variables and meteor packages, as they are global scoped and removing then can be troublesome. --- tools/isobuild/bundler.js | 33 +++++++++++++-- tools/isobuild/import-scanner.ts | 4 ++ tools/isobuild/isopack.js | 2 + tools/isobuild/js-analyze.js | 73 ++++++++++++++++++++++++++++++++ tools/isobuild/package-api.js | 6 +++ tools/isobuild/package-source.js | 9 ++++ tools/isobuild/source-arch.js | 2 + 7 files changed, 126 insertions(+), 3 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 9fdeaf22990..756e9a5a0dc 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -148,6 +148,8 @@ // somewhere (decoupling target type from architecture) but it can // wait until later. +import { removeUnusedExports } from "./js-analyze"; + var assert = require('assert'); var util = require('util'); var Fiber = require('fibers'); @@ -174,6 +176,7 @@ import { CORDOVA_PLATFORM_VERSIONS } from '../cordova'; import { gzipSync } from "zlib"; import { PackageRegistry } from "../../packages/meteor/define-package.js"; import { optimisticLStatOrNull } from '../fs/optimistic'; +import { sha1 } from "../fs/watch"; const SOURCE_URL_PREFIX = "meteor://\u{1f4bb}app"; @@ -1164,18 +1167,42 @@ class Target { // importMap has all the import { x } from '...' // info from every target/bundle in the build system - const importMap = new Map; + const importMap = new Map(); sourceBatches.forEach((sourceBatch) => { const unibuild = sourceBatch.unibuild; const name = unibuild.pkg.name || null; jsOutputFilesMap.get(name).files.forEach((file) => { - file.imports.forEach(([key,object]) => { - importMap.set(key, [...importMap.get(key), ...object]) + if (!file.imports) return; + Object.entries(file.imports).forEach(([key, object]) => { + importMap.set(key, { + deps: [...new Set([...(importMap.get(key)?.deps || []), ...object.deps])], + sideEffects: object.sideEffects + }) }) }) }) // now we need to remove the exports, and let the minifier do it's job later + sourceBatches.forEach((sourceBatch) => { + const unibuild = sourceBatch.unibuild; + + console.log(unibuild.pkg.sideEffects); + const name = unibuild.pkg.name || null; + // we will assume that every meteor package that exports something is a + // side effect true package + if(unibuild.declaredExports.length){ + return; + } + jsOutputFilesMap.get(name).files.forEach((file) => { + const newVar = importMap.get(file.absPath) + const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, file.hash, newVar); + if(newSource) { + file.dataString = newSource; + file.data = Buffer.from(newSource, "utf8"); + file.hash = sha1(file.data); + } + }) + }) // Copy their resources into the bundle in order diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 3787bdf0dca..5aeb065be7c 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -1127,6 +1127,10 @@ export default class ImportScanner { if (! absImportedPath) { return; } + if(file.imports && file.imports[id]){ + file.imports[absImportedPath] = file.imports[id]; + delete file.imports[id] + } let depFile = this.getFile(absImportedPath); diff --git a/tools/isobuild/isopack.js b/tools/isobuild/isopack.js index 696ff871541..46b210406e4 100644 --- a/tools/isobuild/isopack.js +++ b/tools/isobuild/isopack.js @@ -49,6 +49,7 @@ var Isopack = function () { // These have the same meaning as in PackageSource. self.name = null; + self.sideEffects = true; self.metadata = {}; self.version = null; self.isTest = false; @@ -256,6 +257,7 @@ _.extend(Isopack.prototype, { var self = this; self.name = options.name; self.metadata = options.metadata; + self.sideEffects = options.sideEffects; self.version = options.version; self.isTest = options.isTest; self.plugins = options.plugins; diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index edf8eeba2a9..f16654b7be1 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -217,6 +217,79 @@ const importedIdentifierVisitor = new (class extends Visitor { } }); +export function removeUnusedExports(source, hash, exportInfo) { + const possibleIndexes = findPossibleIndexes(source, [ + "export", + ]); + + if (possibleIndexes.length === 0) { + return {}; + } + + const ast = tryToParse(source, hash); + const removeUnusedExportsVisitor = new RemoveUnusedExportsVisitor(exportInfo); + removeUnusedExportsVisitor.visit(ast, source, possibleIndexes); + const newSource = require('@babel/generator').default(ast).code; + return { source:newSource, madeChanges: removeUnusedExportsVisitor.madeChanges }; +} + +class RemoveUnusedExportsVisitor extends Visitor { + constructor(exportInfo) { + super(); + this.exportInfo = exportInfo; + this.madeChanges = false; + } + reset(rootPath, code, possibleIndexes) { + + // Defining this.possibleIndexes causes the Visitor to ignore any + // subtrees of the AST that do not contain any indexes of identifiers + // that we care about. Note that findPossibleIndexes uses a RegExp to + // scan for the given identifiers, so there may be false positives, + // but that's fine because it just means scanning more of the AST. + this.possibleIndexes = possibleIndexes; + } + + removeIdentifiers(node) { + node.properties = node.properties.map((property) => { + if(this.exportInfo && this.exportInfo.sideEffects){ + return property; + } + + const exportKey = property.key.value || property.key.name; + let returnValue = (this.exportInfo?.deps || []).includes(exportKey) ? property : null; + if(!returnValue){ + console.log(`Removing ${exportKey}`) + } + return returnValue; + }).filter(Boolean) + this.madeChanges = true; + } + + visitCallExpression(path) { + const node = path.getValue(); + const args = node.arguments; + + this.visitChildren(path); + + if (node.callee.type === "MemberExpression" && + // The Reify compiler sometimes renames references to the + // CommonJS module object for hygienic purposes, but it + // always does so by appending additional numbers. + isIdWithName(node.callee.object, /^module\d*$/)) { + const propertyName = + isPropertyWithName(node.callee.property, "export"); + + if (propertyName) { + const firstArg = args[0]; + // if we have an object definition on module.export() + if(firstArg) { + this.removeIdentifiers(firstArg); + } + } + } + } +} + function isIdWithName(node, name) { if (! node || node.type !== "Identifier") { diff --git a/tools/isobuild/package-api.js b/tools/isobuild/package-api.js index ec6fcb401f5..148800b5487 100644 --- a/tools/isobuild/package-api.js +++ b/tools/isobuild/package-api.js @@ -85,6 +85,8 @@ export class PackageAPI { // symbols exported this.exports = {}; + this.sideEffects = true; + // packages used and implied (keys are 'package', 'unordered', and // 'weak'). an "implied" package is a package that will be used by a unibuild // which uses us. @@ -420,6 +422,10 @@ export class PackageAPI { this._addFiles("assets", paths, arch); } + setSideEffects(sideEffects){ + this.sideEffects = sideEffects; + } + /** * Internal method used by addFiles and addAssets. */ diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js index 253c5fda18f..80607e86e61 100644 --- a/tools/isobuild/package-source.js +++ b/tools/isobuild/package-source.js @@ -288,6 +288,9 @@ var PackageSource = function () { // a package. self.name = null; + + self.sideEffects = true; + // The path relative to which all source file paths are interpreted // in this package. Also used to compute the location of the // package's .npm directory (npm shrinkwrap state). @@ -456,6 +459,7 @@ _.extend(PackageSource.prototype, { const sourceArch = new SourceArch(self, { kind: options.kind, arch: "os", + sideEffects: self.sideEffects, sourceRoot: self.sourceRoot, uses: _.map(options.use, splitConstraint), getFiles() { @@ -853,10 +857,12 @@ _.extend(PackageSource.prototype, { return result; }, declaredExports: api.exports[arch], + sideEffects: api.sideEffects, watchSet: watchSet })); }); + self.sideEffects = api.sideEffects; // Serve root of the package. self.serveRoot = files.pathJoin('/packages/', self.name); @@ -930,6 +936,8 @@ _.extend(PackageSource.prototype, { const nodeModulesToRecompile = projectContext.meteorConfig .getNodeModulesToRecompile(arch, nodeModulesToRecompileByArch); + const sideEffects = JSON.parse(files.readFile(projectContext.meteorConfig.packageJsonPath)).sideEffects || false; + console.log(`Reading side effects: ${sideEffects}`); // XXX what about /web.browser/* etc, these directories could also // be for specific client targets. @@ -939,6 +947,7 @@ _.extend(PackageSource.prototype, { arch: arch, sourceRoot: self.sourceRoot, uses: uses, + sideEffects, getFiles(sourceProcessorSet, watchSet) { sourceProcessorSet.watchSet = watchSet; diff --git a/tools/isobuild/source-arch.js b/tools/isobuild/source-arch.js index e51cf4c1478..3040940b3b8 100644 --- a/tools/isobuild/source-arch.js +++ b/tools/isobuild/source-arch.js @@ -22,6 +22,7 @@ export default class SourceArch { // Do not include the source files in watchSet. They will be added at // compile time when the sources are actually read. watchSet = new WatchSet(), + sideEffects = true, }) { isString(kind) || reportMissingOption('kind'); isString(arch) || reportMissingOption('arch'); @@ -29,6 +30,7 @@ export default class SourceArch { isFunction(getFiles) || reportMissingOption('getFiles'); this.pkg = pkg; + this.sideEffects = sideEffects; // Kind of this sourceArchitecture. At the moment, there are really // three options -- package, plugin, and app. We use these in linking. From 90e39f04494c2602e4bf8a5415ff067b84f94113 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 10 Sep 2020 17:38:46 -0300 Subject: [PATCH 08/35] Add api for setting sideEffects on meteor modules, start the changes on resolver based on symbols. --- packages/base64/package.js | 2 ++ packages/ejson/ejson.js | 2 +- packages/modules/package.js | 1 + tools/isobuild/bundler.js | 15 ++++++++++++--- tools/isobuild/compiler-plugin.js | 4 ++++ tools/isobuild/compiler.js | 5 +++++ tools/isobuild/import-scanner.ts | 1 + tools/isobuild/isopack-cache.js | 4 ++++ tools/isobuild/isopack.js | 6 ++++++ tools/isobuild/js-analyze.js | 21 ++++++++++++++++++--- tools/isobuild/package-api.js | 1 + tools/isobuild/package-source.js | 4 ++-- tools/isobuild/resolver.ts | 2 +- tools/packaging/catalog/catalog-local.js | 6 ++++++ tools/project-context.js | 1 - 15 files changed, 64 insertions(+), 11 deletions(-) diff --git a/packages/base64/package.js b/packages/base64/package.js index 547992b2a4c..061af890602 100644 --- a/packages/base64/package.js +++ b/packages/base64/package.js @@ -6,10 +6,12 @@ Package.describe({ Package.onUse(api => { api.export('Base64'); api.use('ecmascript'); + api.setSideEffects(false); api.mainModule('base64.js'); }); Package.onTest(api => { api.use(['ecmascript', 'tinytest', 'ejson']); + api.setSideEffects(false); api.addFiles('base64_test.js', ['client', 'server']); }); diff --git a/packages/ejson/ejson.js b/packages/ejson/ejson.js index a4670297d28..7e4ff25dd8f 100644 --- a/packages/ejson/ejson.js +++ b/packages/ejson/ejson.js @@ -9,7 +9,7 @@ import { isInfOrNaN, handleError, } from './utils'; - +import Base64 from 'meteor/base64'; /** * @namespace * @summary Namespace for EJSON functions diff --git a/packages/modules/package.js b/packages/modules/package.js index b697b26ec12..9f6404107c3 100644 --- a/packages/modules/package.js +++ b/packages/modules/package.js @@ -15,6 +15,7 @@ Package.onUse(function(api) { api.mainModule("client.js", "client"); api.mainModule("server.js", "server"); api.export("meteorInstall"); + api.setSideEffects(false); // When compiling legacy code, the babel-compiler and meteor-babel // packages assume meteorBabelHelpers.sanitizeForInObject is defined. diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 756e9a5a0dc..a998ae81790 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1185,15 +1185,23 @@ class Target { // now we need to remove the exports, and let the minifier do it's job later sourceBatches.forEach((sourceBatch) => { const unibuild = sourceBatch.unibuild; - - console.log(unibuild.pkg.sideEffects); const name = unibuild.pkg.name || null; + + if(name === 'modules') { + debugger; + console.log(`sideEffects for: ${name} ${unibuild.pkg.sideEffects}`); + } // we will assume that every meteor package that exports something is a // side effect true package - if(unibuild.declaredExports.length){ + if(unibuild.pkg.sideEffects){ return; } + if(name === 'modules'){ + // console.log(importMap); + } + jsOutputFilesMap.get(name).files.forEach((file) => { + const newVar = importMap.get(file.absPath) const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, file.hash, newVar); if(newSource) { @@ -3528,6 +3536,7 @@ exports.buildJsImage = Profile("bundler.buildJsImage", function (options) { npmDependencies: options.npmDependencies, npmDir: options.npmDir, localNodeModulesDirs: options.localNodeModulesDirs, + sideEffects: options.sideEffects, }); var isopack = compiler.compile(packageSource, { diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index b8b476d9c41..eefa054226a 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -145,6 +145,10 @@ export class CompilerPluginProcessor { var sourceProcessorsWithSlots = {}; var sourceBatches = _.map(self.unibuilds, function (unibuild) { + if(unibuild.pkg.name === 'base64') { + console.log(`runCompilerPlugins -> ${unibuild.pkg.name} ${unibuild.pkg.sideEffects}`); + } + const { pkg: { name }, arch } = unibuild; const sourceRoot = name && self.isopackCache.getSourceRoot(name, arch) diff --git a/tools/isobuild/compiler.js b/tools/isobuild/compiler.js index 3ecdb82d614..08d4dde3b0c 100644 --- a/tools/isobuild/compiler.js +++ b/tools/isobuild/compiler.js @@ -72,6 +72,7 @@ compiler.compile = Profile(function (packageSource, options) { isopackCache: isopackCache, use: info.use, sourceRoot: packageSource.sourceRoot, + sideEffects: packageSource.sideEffects, sources: info.sources, // While we're not actually "serving" the file, the serveRoot is used to // calculate file names in source maps. @@ -174,6 +175,7 @@ compiler.compile = Profile(function (packageSource, options) { prodOnly: packageSource.prodOnly, testOnly: packageSource.testOnly, pluginCacheDir: options.pluginCacheDir, + sideEffects: packageSource.sideEffects, isobuildFeatures }); @@ -956,6 +958,9 @@ compiler.eachUsedUnibuild = function ( } var usedPackage = isopackCache.getIsopack(use.package); + if(usedPackage.name === 'base64') { + console.log(`eachUsedUnibuild - isopack -> ${usedPackage.sideEffects}`); + } // Ignore this package if we were told to skip debug-only packages and it is // debug-only. diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 5aeb065be7c..cafc94682e0 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -1123,6 +1123,7 @@ export default class ImportScanner { info.dynamic); const resolved = this.resolve(file, id, dynamic); + console.log(resolved && resolved.path); const absImportedPath = resolved && resolved !== "missing" && resolved.path; if (! absImportedPath) { return; diff --git a/tools/isobuild/isopack-cache.js b/tools/isobuild/isopack-cache.js index 06e27fd5de7..68860edcf8d 100644 --- a/tools/isobuild/isopack-cache.js +++ b/tools/isobuild/isopack-cache.js @@ -209,6 +209,10 @@ export class IsopackCache { }; var packageInfo = self._packageMap.getInfo(name); + if(name === 'base64') { + console.log(`ensurePackageLoaded - isopackCache -> ${packageInfo}`); + } + if (! packageInfo) { throw Error("Depend on unknown package " + name + "?"); } diff --git a/tools/isobuild/isopack.js b/tools/isobuild/isopack.js index 46b210406e4..7c49a56a9d6 100644 --- a/tools/isobuild/isopack.js +++ b/tools/isobuild/isopack.js @@ -890,6 +890,7 @@ _.extend(Isopack.prototype, { }; self.version = mainJson.version; self.isTest = mainJson.isTest; + self.sideEffects = mainJson.sideEffects; self.debugOnly = !!mainJson.debugOnly; self.prodOnly = !!mainJson.prodOnly; self.testOnly = !!mainJson.testOnly; @@ -946,6 +947,7 @@ _.extend(Isopack.prototype, { }); self.cordovaDependencies = mainJson.cordovaDependencies || null; + self.sideEffects = mainJson.sideEffects !== false; _.each(mainJson.tools, function (toolMeta) { toolMeta.rootDir = dir; @@ -1021,12 +1023,16 @@ _.extend(Isopack.prototype, { var outputPath = outputDir; var builder = new Builder({ outputPath: outputPath }); + if(self.name === 'base64') { + console.log(`saveToPath - isopack -> ${self.sideEffects}`); + } try { var mainJson = { name: self.name, summary: self.metadata.summary, version: self.version, isTest: self.isTest, + sideEffects: self.sideEffects, builds: [], plugins: [] }; diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index f16654b7be1..549d2b2ece5 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -220,6 +220,7 @@ const importedIdentifierVisitor = new (class extends Visitor { export function removeUnusedExports(source, hash, exportInfo) { const possibleIndexes = findPossibleIndexes(source, [ "export", + "exportDefault", ]); if (possibleIndexes.length === 0) { @@ -249,7 +250,20 @@ class RemoveUnusedExportsVisitor extends Visitor { this.possibleIndexes = possibleIndexes; } - removeIdentifiers(node) { + removeIdentifiers(exportPath, node, defaultExport = false) { + if(defaultExport && + !this.exportInfo?.deps.includes('default') && + !this.exportInfo.sideEffects){ + exportPath.replace({ + type: "BooleanLiteral", + value: false, + }) + console.log(`Removing Default export`) + this.madeChanges = true; + return; + }else if(defaultExport){ + return; + } node.properties = node.properties.map((property) => { if(this.exportInfo && this.exportInfo.sideEffects){ return property; @@ -277,13 +291,14 @@ class RemoveUnusedExportsVisitor extends Visitor { // always does so by appending additional numbers. isIdWithName(node.callee.object, /^module\d*$/)) { const propertyName = - isPropertyWithName(node.callee.property, "export"); + isPropertyWithName(node.callee.property, "export") || + isPropertyWithName(node.callee.property, "exportDefault"); if (propertyName) { const firstArg = args[0]; // if we have an object definition on module.export() if(firstArg) { - this.removeIdentifiers(firstArg); + this.removeIdentifiers(path, firstArg, isPropertyWithName(node.callee.property, "exportDefault")); } } } diff --git a/tools/isobuild/package-api.js b/tools/isobuild/package-api.js index 148800b5487..e8451ac6562 100644 --- a/tools/isobuild/package-api.js +++ b/tools/isobuild/package-api.js @@ -424,6 +424,7 @@ export class PackageAPI { setSideEffects(sideEffects){ this.sideEffects = sideEffects; + console.log(`Setting side effects ${this.sideEffects}`); } /** diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js index 80607e86e61..d8b82d5d2a4 100644 --- a/tools/isobuild/package-source.js +++ b/tools/isobuild/package-source.js @@ -688,7 +688,6 @@ _.extend(PackageSource.prototype, { Cordova._dependencies = null; } } - // By the way, you can't depend on yourself. var doNotDepOnSelf = function (dep) { if (dep.package === self.name) { @@ -862,7 +861,6 @@ _.extend(PackageSource.prototype, { })); }); - self.sideEffects = api.sideEffects; // Serve root of the package. self.serveRoot = files.pathJoin('/packages/', self.name); @@ -870,6 +868,8 @@ _.extend(PackageSource.prototype, { if (Package._hasTests) { self.testName = genTestName(self.name); } + self.sideEffects = api.sideEffects; + }), _readAndWatchDirectory(relDir, watchSet, {include, exclude, names}) { diff --git a/tools/isobuild/resolver.ts b/tools/isobuild/resolver.ts index 0092f20fb41..45d49f7c19e 100644 --- a/tools/isobuild/resolver.ts +++ b/tools/isobuild/resolver.ts @@ -105,7 +105,7 @@ export default class Resolver { // import/export in their "module" dependency trees. this.mainFields = ["browser", "main", "module"]; } else { - this.mainFields = ["browser", "module", "main"]; + this.mainFields = ["module", "browser", "main"]; } } else { this.mainFields = ["main"]; diff --git a/tools/packaging/catalog/catalog-local.js b/tools/packaging/catalog/catalog-local.js index 3368108aa10..827ee6e950d 100644 --- a/tools/packaging/catalog/catalog-local.js +++ b/tools/packaging/catalog/catalog-local.js @@ -352,6 +352,9 @@ _.extend(LocalCatalog.prototype, { initFromPackageDirOptions.name = definiteName; } packageSource.initFromPackageDir(packageDir, initFromPackageDirOptions); + if(packageSource.name === 'base64'){ + console.log(`initSourceFromDir -> ${packageSource.sideEffects}`); + } if (buildmessage.jobHasMessages()) return; // recover by ignoring @@ -415,6 +418,9 @@ _.extend(LocalCatalog.prototype, { var self = this; if (! _.has(self.packages, name)) return null; + if(name === 'base64'){ + console.log(`getPackageSource -> ${self.packages[name].packageSource.sideEffects}`); + } return self.packages[name].packageSource; } }); diff --git a/tools/project-context.js b/tools/project-context.js index 467c1e48bb6..3411f32eb65 100644 --- a/tools/project-context.js +++ b/tools/project-context.js @@ -581,7 +581,6 @@ _.extend(ProjectContext.prototype, { self.packageMap = new packageMapModule.PackageMap(solution.answer, { localCatalog: self.localCatalog }); - self.packageMapDelta = new packageMapModule.PackageMapDelta({ cachedVersions: cachedVersions, packageMap: self.packageMap, From 483029e579df2d67caf8171db150493ca0fb8947 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Fri, 11 Sep 2020 18:08:19 -0300 Subject: [PATCH 09/35] Using parent for deciding which files to include in the bundle in a case of a proxy export, now files are also "tree shaked", as well as export declarations. Working with lodash-es. --- tools/isobuild/bundler.js | 12 +- tools/isobuild/compiler-plugin.js | 5 +- tools/isobuild/import-scanner.ts | 46 +- tools/isobuild/js-analyze.js | 710 ++++++++++++++++-------------- tools/isobuild/package-api.js | 4 + 5 files changed, 437 insertions(+), 340 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index a998ae81790..9ffe7ec1c31 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1182,6 +1182,9 @@ class Target { }) }) + const allFilesOnBundle = new Set(Array.from(jsOutputFilesMap).flatMap(([, value]) => { + return value.files.map(({absPath}) => absPath); + })); // now we need to remove the exports, and let the minifier do it's job later sourceBatches.forEach((sourceBatch) => { const unibuild = sourceBatch.unibuild; @@ -1199,11 +1202,14 @@ class Target { if(name === 'modules'){ // console.log(importMap); } - jsOutputFilesMap.get(name).files.forEach((file) => { + const importedSymbolsFromFile = importMap.get(file.absPath) - const newVar = importMap.get(file.absPath) - const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, file.hash, newVar); + const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, + file.hash, + importedSymbolsFromFile, + allFilesOnBundle, + file.resolveMap || new Map()); if(newSource) { file.dataString = newSource; file.data = Buffer.from(newSource, "utf8"); diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index eefa054226a..21f81a018fe 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -1266,7 +1266,10 @@ export class PackageSourceBatch { }); map.set(name, { - files: inputFiles, + files: inputFiles.map(file => { + file.sideEffects = batch.unibuild.pkg.sideEffects; + return file; + }), mainModule: _.find(inputFiles, file => file.mainModule) || null, batch, importScannerWatchSet: new WatchSet(), diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index cafc94682e0..00e1e780b0e 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -333,14 +333,21 @@ interface RawFile { jsonData?: Record; } +interface ImportIdentifiersEntry { + deps: [string]; + sideEffects: boolean; +} + interface File extends RawFile { type: string; + sideEffects: boolean; sourcePath: string; targetPath?: string; servePath?: string; absModuleId?: string; deps?: Record; - imports?: Record; + resolveMap?: Map; + imports?: Record; lazy: boolean; bare?: boolean; // TODO Improve the sourceMap type. @@ -371,7 +378,7 @@ type ImportInfo = { helpers?: Record; } type JSAnalyzeInfo = { - dependencies?: Record; + dependencies?: Record; identifiers?: Record; } @@ -981,6 +988,7 @@ export default class ImportScanner { private findImportedModuleIdentifiers( file: File, + parentImportSymbols?: ImportIdentifiersEntry ): JSAnalyzeInfo { if (IMPORT_SCANNER_CACHE.has(file.hash)) { return IMPORT_SCANNER_CACHE.get(file.hash); @@ -989,6 +997,8 @@ export default class ImportScanner { const result = findImportedModuleIdentifiers( this.getDataString(file), file.hash, + parentImportSymbols, + file.sideEffects ); // there should always be file.hash, but better safe than sorry @@ -1048,7 +1058,14 @@ export default class ImportScanner { return file.alias; } } - + if(resolved) { + const file = this.getFile(resolved.path); + if(!file) return resolved; + if (!file.resolveMap) { + file.resolveMap = new Map(); + } + file.resolveMap.set(id, resolved.path); + } return resolved; } @@ -1068,7 +1085,7 @@ export default class ImportScanner { return relativeId; } - private scanFile(file: File, forDynamicImport = false) { + private scanFile(file: File, forDynamicImport = false, parentImportSymbols?: ImportIdentifiersEntry) { if (file.imported === "static") { // If we've already scanned this file non-dynamically, then we don't // need to scan it again. @@ -1097,7 +1114,7 @@ export default class ImportScanner { try { let importInfo; if(!file.deps){ - importInfo = this.findImportedModuleIdentifiers(file); + importInfo = this.findImportedModuleIdentifiers(file, parentImportSymbols); } file.deps = file.deps || importInfo?.identifiers; file.imports = file.imports || importInfo?.dependencies; @@ -1123,7 +1140,6 @@ export default class ImportScanner { info.dynamic); const resolved = this.resolve(file, id, dynamic); - console.log(resolved && resolved.path); const absImportedPath = resolved && resolved !== "missing" && resolved.path; if (! absImportedPath) { return; @@ -1133,6 +1149,11 @@ export default class ImportScanner { delete file.imports[id] } + if(!file.resolveMap){ + file.resolveMap = new Map(); + } + file.resolveMap.set(id, absImportedPath); + let depFile = this.getFile(absImportedPath); if (depFile) { @@ -1156,7 +1177,11 @@ export default class ImportScanner { // If depFile has already been scanned, this._scanFile will return // immediately thanks to the depFile.imported-checking logic at // the top of the method. - this.scanFile(depFile, dynamic); + if(file.imports && file.imports[depFile.absPath]){ + this.scanFile(depFile, dynamic, file.imports[depFile.absPath]); + }else{ + this.scanFile(depFile, dynamic); + } return; } @@ -1169,8 +1194,11 @@ export default class ImportScanner { // Append this file to the output array and record its index. this.addFile(absImportedPath, depFile); - // Recursively scan the module's imported dependencies. - this.scanFile(depFile, dynamic); + if(file.imports && file.imports[depFile.absPath]){ + this.scanFile(depFile, dynamic, file.imports[depFile.absPath]); + }else{ + this.scanFile(depFile, dynamic); + } }); } diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index 549d2b2ece5..48822a98253 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -1,46 +1,46 @@ -import { parse } from 'meteor-babel'; -import { analyze as analyzeScope } from 'escope'; +import {parse} from 'meteor-babel'; +import {analyze as analyzeScope} from 'escope'; import LRU from "lru-cache"; import Visitor from "reify/lib/visitor.js"; -import { findPossibleIndexes } from "reify/lib/utils.js"; +import {findPossibleIndexes} from "reify/lib/utils.js"; const hasOwn = Object.prototype.hasOwnProperty; const objToStr = Object.prototype.toString function isRegExp(value) { - return value && objToStr.call(value) === "[object RegExp]"; + return value && objToStr.call(value) === "[object RegExp]"; } var AST_CACHE = new LRU({ - max: Math.pow(2, 12), - length(ast) { - return ast.loc.end.line; - } + max: Math.pow(2, 12), + length(ast) { + return ast.loc.end.line; + } }); // Like babel.parse, but annotates any thrown error with $ParseError = true. function tryToParse(source, hash) { - if (hash && AST_CACHE.has(hash)) { - return AST_CACHE.get(hash); - } + if (hash && AST_CACHE.has(hash)) { + return AST_CACHE.get(hash); + } - let ast; + let ast; - try { - ast = parse(source); - } catch (e) { - if (typeof e.loc === 'object') { - e.$ParseError = true; + try { + ast = parse(source); + } catch (e) { + if (typeof e.loc === 'object') { + e.$ParseError = true; + } + throw e; } - throw e; - } - if (hash) { - AST_CACHE.set(hash, ast); - } + if (hash) { + AST_CACHE.set(hash, ast); + } - return ast; + return ast; } /** @@ -57,284 +57,340 @@ function tryToParse(source, hash) { * if the tokens were actually what we thought they were (a `require` * function call, or an `import` or `export` statement). */ -export function findImportedModuleIdentifiers(source, hash) { - const possibleIndexes = findPossibleIndexes(source, [ - "require", - "import", - "export", - "dynamicImport", - "link", - ]); - - if (possibleIndexes.length === 0) { - return {}; - } - - const ast = tryToParse(source, hash); - importedIdentifierVisitor.visit(ast, source, possibleIndexes); - return { identifiers:importedIdentifierVisitor.identifiers, dependencies:importedIdentifierVisitor.dependencies }; +export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, hasSideEffects) { + const possibleIndexes = findPossibleIndexes(source, [ + "require", + "import", + "export", + "dynamicImport", + "link", + ]); + + if (possibleIndexes.length === 0) { + return {}; + } + + const ast = tryToParse(source, hash); + importedIdentifierVisitor.visit(ast, source, possibleIndexes, parentImportSymbols, hasSideEffects); + return {identifiers: importedIdentifierVisitor.identifiers, dependencies: importedIdentifierVisitor.dependencies}; } const importedIdentifierVisitor = new (class extends Visitor { - reset(rootPath, code, possibleIndexes) { - this.requireIsBound = false; - this.identifiers = Object.create(null); - this.dependencies = Object.create(null); - - // Defining this.possibleIndexes causes the Visitor to ignore any - // subtrees of the AST that do not contain any indexes of identifiers - // that we care about. Note that findPossibleIndexes uses a RegExp to - // scan for the given identifiers, so there may be false positives, - // but that's fine because it just means scanning more of the AST. - this.possibleIndexes = possibleIndexes; - } - - addIdentifier(id, type, dynamic) { - const entry = hasOwn.call(this.identifiers, id) - ? this.identifiers[id] - : this.identifiers[id] = { - possiblySpurious: true, - dynamic: !! dynamic - }; - - if (! dynamic) { - entry.dynamic = false; + reset(rootPath, code, possibleIndexes, parentImportSymbols, hasSideEffects = false) { + this.parentImportSymbols = parentImportSymbols || []; + this.hasSideEffects = hasSideEffects; + this.requireIsBound = false; + this.identifiers = Object.create(null); + this.dependencies = Object.create(null); + + // Defining this.possibleIndexes causes the Visitor to ignore any + // subtrees of the AST that do not contain any indexes of identifiers + // that we care about. Note that findPossibleIndexes uses a RegExp to + // scan for the given identifiers, so there may be false positives, + // but that's fine because it just means scanning more of the AST. + this.possibleIndexes = possibleIndexes; + } + + addIdentifier(id, type, dynamic) { + const entry = hasOwn.call(this.identifiers, id) + ? this.identifiers[id] + : this.identifiers[id] = { + possiblySpurious: true, + dynamic: !!dynamic + }; + + if (!dynamic) { + entry.dynamic = false; + } + + if (type === "require") { + // If the identifier comes from a require call, but require is not a + // free variable, then this dependency might be spurious. + entry.possiblySpurious = + entry.possiblySpurious && this.requireIsBound; + } else { + // The import keyword can't be shadowed, so any dependencies + // registered by import statements should be trusted absolutely. + entry.possiblySpurious = false; + } } - if (type === "require") { - // If the identifier comes from a require call, but require is not a - // free variable, then this dependency might be spurious. - entry.possiblySpurious = - entry.possiblySpurious && this.requireIsBound; - } else { - // The import keyword can't be shadowed, so any dependencies - // registered by import statements should be trusted absolutely. - entry.possiblySpurious = false; + addDependency(id, importIdentifiers, sideEffects) { + this.dependencies[id] = this.dependencies[id] ? + { + deps: [...new Set([...this.dependencies[id].deps, ...importIdentifiers])], + sideEffects: this.dependencies[id].sideEffects || sideEffects, + } : + { + deps: importIdentifiers, + sideEffects + }; } - } - - addDependency(id, importIdentifiers, sideEffects) { - this.dependencies[id] = this.dependencies[id] ? - { - deps: new Set([...this.dependencies[id].deps, ...importIdentifiers]), - sideEffects: this.dependencies[id].sideEffects || sideEffects, - } : - { - deps: importIdentifiers, - sideEffects - }; - } - - visitFunctionExpression(path) { - return this._functionParamRequireHelper(path); - } - - visitFunctionDeclaration(path) { - return this._functionParamRequireHelper(path); - } - - visitArrowFunctionExpression(path) { - return this._functionParamRequireHelper(path); - } - - _functionParamRequireHelper(path) { - const node = path.getValue(); - if (node.params.some(param => isIdWithName(param, "require"))) { - const { requireIsBound } = this; - this.requireIsBound = true; - this.visitChildren(path); - this.requireIsBound = requireIsBound; - } else { - this.visitChildren(path); + + visitFunctionExpression(path) { + return this._functionParamRequireHelper(path); } - } - visitCallExpression(path) { - const node = path.getValue(); - const args = node.arguments; - const firstArg = args[0]; + visitFunctionDeclaration(path) { + return this._functionParamRequireHelper(path); + } - this.visitChildren(path); + visitArrowFunctionExpression(path) { + return this._functionParamRequireHelper(path); + } - if (! isStringLiteral(firstArg)) { - return; + _functionParamRequireHelper(path) { + const node = path.getValue(); + if (node.params.some(param => isIdWithName(param, "require"))) { + const {requireIsBound} = this; + this.requireIsBound = true; + this.visitChildren(path); + this.requireIsBound = requireIsBound; + } else { + this.visitChildren(path); + } } - if (isIdWithName(node.callee, "require")) { - this.addIdentifier(firstArg.value, "require"); - } else if (node.callee.type === "Import" || - isIdWithName(node.callee, "import")) { - this.addIdentifier(firstArg.value, "import", true); - } else if (node.callee.type === "MemberExpression" && - // The Reify compiler sometimes renames references to the - // CommonJS module object for hygienic purposes, but it - // always does so by appending additional numbers. - isIdWithName(node.callee.object, /^module\d*$/)) { - const propertyName = - isPropertyWithName(node.callee.property, "link") || - isPropertyWithName(node.callee.property, "dynamicImport"); - - if (propertyName) { - // if we have an object definition on module.link(), we are importing with ES6 possible without side effects - // otherwise we are considering it as a side effect import - if(args[1]) { - this.addDependency(firstArg.value, args[1].properties.map(({key}) => key.name), false); - }else{ - this.addDependency(firstArg.value, [], true); + visitCallExpression(path) { + const node = path.getValue(); + const args = node.arguments; + const firstArg = args[0]; + + this.visitChildren(path); + + if (!isStringLiteral(firstArg)) { + return; + } + + if (isIdWithName(node.callee, "require")) { + this.addIdentifier(firstArg.value, "require"); + this.addDependency(firstArg.value, [], true); + } else if (node.callee.type === "Import" || + isIdWithName(node.callee, "import")) { + this.addIdentifier(firstArg.value, "import", true); + this.addDependency(firstArg.value, [], true); + } else if (node.callee.type === "MemberExpression" && + // The Reify compiler sometimes renames references to the + // CommonJS module object for hygienic purposes, but it + // always does so by appending additional numbers. + isIdWithName(node.callee.object, /^module\d*$/)) { + const propertyName = + isPropertyWithName(node.callee.property, "link") || + isPropertyWithName(node.callee.property, "dynamicImport"); + + if (propertyName) { + // if we have an object definition on module.link(), we are importing with ES6 possible without side effects + // otherwise we are considering it as a side effect import + if (args.length <= 1) { + this.addDependency(firstArg.value, [], true); + this.addIdentifier( + firstArg.value, + "import", + propertyName === "dynamicImport" + ); + return; + } + const secondArg = args[1]; + // if every prop is an string literal, we have something like: export { a, b, c } from './teste'; + if (secondArg.properties.every(({value}) => isStringLiteral(value)) && !this.hasSideEffects) { + // in this case, we need to verify the parent imports to make sure we follow the right tree path + const isImportedByParent = secondArg.properties.some(({value}) => { + if(this.parentImportSymbols){ + // console.log(this.parentImportSymbols); + // console.log(value.value); + } + return this.parentImportSymbols.deps.includes(value.value) || this.parentImportSymbols.deps.includes("*"); + }); + const isSomeSymbolImported = this.parentImportSymbols ? + this.parentImportSymbols.sideEffects || isImportedByParent + : true; + // console.log(isImportedByParent); + // console.log(this.parentImportSymbols); + + + if (isSomeSymbolImported) { + this.addIdentifier( + firstArg.value, + "import", + propertyName === "dynamicImport" + ); + } + this.addDependency(firstArg.value, secondArg.properties.map(({key, value}) => this.parentImportSymbols.deps.includes(value.value) && key.name).filter(Boolean), false); + console.log(firstArg.value); + return; + } + this.addIdentifier( + firstArg.value, + "import", + propertyName === "dynamicImport" + ); + this.addDependency(firstArg.value, secondArg.properties.map(({key}) => key.name), false); + + } } - this.addIdentifier( - firstArg.value, - "import", - propertyName === "dynamicImport" - ); - } } - } - - visitImportDeclaration(path) { - return this._importExportSourceHelper(path); - } - - visitExportAllDeclaration(path) { - return this._importExportSourceHelper(path); - } - - visitExportNamedDeclaration(path) { - return this._importExportSourceHelper(path); - } - - _importExportSourceHelper(path) { - const node = path.getValue(); - // The .source of an ImportDeclaration or Export{Named,All}Declaration - // is always a string-valued Literal node, if not null. - if (isStringLiteral(node.source)) { - this.addIdentifier( - node.source.value, - "import", - false - ); + + visitImportDeclaration(path) { + return this._importExportSourceHelper(path); + + } + + visitExportAllDeclaration(path) { + return this._importExportSourceHelper(path); + } + + visitExportNamedDeclaration(path) { + return this._importExportSourceHelper(path); + } + + _importExportSourceHelper(path) { + const node = path.getValue(); + // The .source of an ImportDeclaration or Export{Named,All}Declaration + // is always a string-valued Literal node, if not null. + if (isStringLiteral(node.source)) { + this.addIdentifier( + node.source.value, + "import", + false + ); + } } - } }); -export function removeUnusedExports(source, hash, exportInfo) { - const possibleIndexes = findPossibleIndexes(source, [ - "export", - "exportDefault", - ]); - - if (possibleIndexes.length === 0) { - return {}; - } - - const ast = tryToParse(source, hash); - const removeUnusedExportsVisitor = new RemoveUnusedExportsVisitor(exportInfo); - removeUnusedExportsVisitor.visit(ast, source, possibleIndexes); - const newSource = require('@babel/generator').default(ast).code; - return { source:newSource, madeChanges: removeUnusedExportsVisitor.madeChanges }; +export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = new Set(), resolveMap) { + const possibleIndexes = findPossibleIndexes(source, [ + "export", + "exportDefault", + "link", + "importDynamic" + ]); + + if (possibleIndexes.length === 0) { + return {}; + } + + const ast = tryToParse(source, hash); + removeUnusedExportsVisitor.visit(ast, source, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap); + const newSource = require('@babel/generator').default(ast).code; + return {source: newSource, madeChanges: removeUnusedExportsVisitor.madeChanges}; } -class RemoveUnusedExportsVisitor extends Visitor { - constructor(exportInfo) { - super(); - this.exportInfo = exportInfo; - this.madeChanges = false; - } - reset(rootPath, code, possibleIndexes) { - - // Defining this.possibleIndexes causes the Visitor to ignore any - // subtrees of the AST that do not contain any indexes of identifiers - // that we care about. Note that findPossibleIndexes uses a RegExp to - // scan for the given identifiers, so there may be false positives, - // but that's fine because it just means scanning more of the AST. - this.possibleIndexes = possibleIndexes; - } - - removeIdentifiers(exportPath, node, defaultExport = false) { - if(defaultExport && - !this.exportInfo?.deps.includes('default') && - !this.exportInfo.sideEffects){ - exportPath.replace({ - type: "BooleanLiteral", - value: false, - }) - console.log(`Removing Default export`) - this.madeChanges = true; - return; - }else if(defaultExport){ - return; +const removeUnusedExportsVisitor = new (class extends Visitor { + + reset(rootPath, code, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap) { + this.madeChanges = false; + this.exportInfo = exportInfo; + this.allFilesOnBundle = allFilesOnBundle; + this.resolveMap = resolveMap; + + // Defining this.possibleIndexes causes the Visitor to ignore any + // subtrees of the AST that do not contain any indexes of identifiers + // that we care about. Note that findPossibleIndexes uses a RegExp to + // scan for the given identifiers, so there may be false positives, + // but that's fine because it just means scanning more of the AST. + this.possibleIndexes = possibleIndexes; } - node.properties = node.properties.map((property) => { - if(this.exportInfo && this.exportInfo.sideEffects){ - return property; - } - - const exportKey = property.key.value || property.key.name; - let returnValue = (this.exportInfo?.deps || []).includes(exportKey) ? property : null; - if(!returnValue){ - console.log(`Removing ${exportKey}`) - } - return returnValue; - }).filter(Boolean) - this.madeChanges = true; - } - - visitCallExpression(path) { - const node = path.getValue(); - const args = node.arguments; - - this.visitChildren(path); - - if (node.callee.type === "MemberExpression" && - // The Reify compiler sometimes renames references to the - // CommonJS module object for hygienic purposes, but it - // always does so by appending additional numbers. - isIdWithName(node.callee.object, /^module\d*$/)) { - const propertyName = - isPropertyWithName(node.callee.property, "export") || - isPropertyWithName(node.callee.property, "exportDefault"); - - if (propertyName) { - const firstArg = args[0]; - // if we have an object definition on module.export() - if(firstArg) { - this.removeIdentifiers(path, firstArg, isPropertyWithName(node.callee.property, "exportDefault")); + + removeIdentifiers(exportPath, node, defaultExport = false) { + if (defaultExport && + !this.exportInfo?.deps.includes('default') && + !this.exportInfo.sideEffects) { + exportPath.replace({ + type: "BooleanLiteral", + value: false, + }) + console.log(`Removing Default export`) + this.madeChanges = true; + return; + } else if (defaultExport) { + return; } - } + node.properties = node.properties.map((property) => { + if (this.exportInfo && this.exportInfo.sideEffects) { + return property; + } + + const exportKey = property.key.value || property.key.name; + let returnValue = (this.exportInfo?.deps || []).includes(exportKey) ? property : null; + if (!returnValue) { + console.log(`Removing ${exportKey}`) + } + return returnValue; + }).filter(Boolean) + this.madeChanges = true; } - } -} + + visitCallExpression(path) { + const node = path.getValue(); + const args = node.arguments; + const firstArg = node.arguments[0]; + + this.visitChildren(path); + + if (node.callee.type === "MemberExpression" && + // The Reify compiler sometimes renames references to the + // CommonJS module object for hygienic purposes, but it + // always does so by appending additional numbers. + isIdWithName(node.callee.object, /^module\d*$/)) { + const isExport = + isPropertyWithName(node.callee.property, "export") || + isPropertyWithName(node.callee.property, "exportDefault"); + + const isImport = + isPropertyWithName(node.callee.property, "link") || + isPropertyWithName(node.callee.property, "dynamicImport"); + + if (isExport) { + // if we have an object definition on module.export() + if (firstArg) { + this.removeIdentifiers(path, firstArg, isPropertyWithName(node.callee.property, "exportDefault")); + } + } + if(isImport){ + const absPath = this.resolveMap.get(firstArg.value); + const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; + + if(!fileIsInBundle) { + path.replace({ + type: "BooleanLiteral", + value: false, + }); + } + } + } + } +}); function isIdWithName(node, name) { - if (! node || - node.type !== "Identifier") { - return false; - } + if (!node || + node.type !== "Identifier") { + return false; + } - if (typeof name === "string") { - return node.name === name; - } + if (typeof name === "string") { + return node.name === name; + } - if (isRegExp(name)) { - return name.test(node.name); - } + if (isRegExp(name)) { + return name.test(node.name); + } - return false; + return false; } function isStringLiteral(node) { - return node && ( - node.type === "StringLiteral" || - (node.type === "Literal" && - typeof node.value === "string")); + return node && ( + node.type === "StringLiteral" || + (node.type === "Literal" && + typeof node.value === "string")); } function isPropertyWithName(node, name) { - if (isIdWithName(node, name) || - (isStringLiteral(node) && - node.value === name)) { - return name; - } + if (isIdWithName(node, name) || + (isStringLiteral(node) && + node.value === name)) { + return name; + } } // Analyze the JavaScript source code `source` and return a dictionary of all @@ -348,69 +404,69 @@ function isPropertyWithName(node, name) { // It only cares about assignments to variables; an assignment to a field on an // object (`Foo.Bar = true`) neither causes `Foo` nor `Foo.Bar` to be returned. const globalsCache = new LRU({ - max: Math.pow(2, 12), - length(globals) { - let sum = 0; - Object.keys(globals).forEach(name => sum += name.length); - return sum; - } + max: Math.pow(2, 12), + length(globals) { + let sum = 0; + Object.keys(globals).forEach(name => sum += name.length); + return sum; + } }); export function findAssignedGlobals(source, hash) { - if (hash && globalsCache.has(hash)) { - return globalsCache.get(hash); - } - - const ast = tryToParse(source, hash); - - // We have to pass ignoreEval; otherwise, the existence of a direct eval call - // causes escope to not bother to resolve references in the eval's scope. - // This is because an eval can pull references inward: - // - // function outer() { - // var i = 42; - // function inner() { - // eval('var i = 0'); - // i; // 0, not 42 - // } - // } - // - // But it can't pull references outward, so for our purposes it is safe to - // ignore. - const scopeManager = analyzeScope(ast, { - ecmaVersion: 6, - sourceType: "module", - ignoreEval: true, - // Ensures we don't treat top-level var declarations as globals. - nodejsScope: true, - }); - - const program = ast.type === "File" ? ast.program : ast; - const programScope = scopeManager.acquire(program); - const assignedGlobals = {}; - - // Passing {sourceType: "module"} to analyzeScope leaves this list - // strangely empty, but {sourceType: "script"} forbids ImportDeclaration - // nodes (because they are only legal in modules. - programScope.implicit.variables.forEach(variable => { - assignedGlobals[variable.name] = true; - }); - - // Fortunately, even with {sourceType: "module"}, the .implicit.left - // array still has all the information we need, as long as we ignore - // global variable references that are not assignments. - programScope.implicit.left.forEach(entry => { - if (entry.identifier && - entry.identifier.type === "Identifier" && - // Only consider identifers that are assigned a value. - entry.writeExpr) { - assignedGlobals[entry.identifier.name] = true; + if (hash && globalsCache.has(hash)) { + return globalsCache.get(hash); } - }); - if (hash) { - globalsCache.set(hash, assignedGlobals); - } + const ast = tryToParse(source, hash); + + // We have to pass ignoreEval; otherwise, the existence of a direct eval call + // causes escope to not bother to resolve references in the eval's scope. + // This is because an eval can pull references inward: + // + // function outer() { + // var i = 42; + // function inner() { + // eval('var i = 0'); + // i; // 0, not 42 + // } + // } + // + // But it can't pull references outward, so for our purposes it is safe to + // ignore. + const scopeManager = analyzeScope(ast, { + ecmaVersion: 6, + sourceType: "module", + ignoreEval: true, + // Ensures we don't treat top-level var declarations as globals. + nodejsScope: true, + }); + + const program = ast.type === "File" ? ast.program : ast; + const programScope = scopeManager.acquire(program); + const assignedGlobals = {}; + + // Passing {sourceType: "module"} to analyzeScope leaves this list + // strangely empty, but {sourceType: "script"} forbids ImportDeclaration + // nodes (because they are only legal in modules. + programScope.implicit.variables.forEach(variable => { + assignedGlobals[variable.name] = true; + }); + + // Fortunately, even with {sourceType: "module"}, the .implicit.left + // array still has all the information we need, as long as we ignore + // global variable references that are not assignments. + programScope.implicit.left.forEach(entry => { + if (entry.identifier && + entry.identifier.type === "Identifier" && + // Only consider identifers that are assigned a value. + entry.writeExpr) { + assignedGlobals[entry.identifier.name] = true; + } + }); + + if (hash) { + globalsCache.set(hash, assignedGlobals); + } - return assignedGlobals; + return assignedGlobals; } diff --git a/tools/isobuild/package-api.js b/tools/isobuild/package-api.js index e8451ac6562..fcf1699f18c 100644 --- a/tools/isobuild/package-api.js +++ b/tools/isobuild/package-api.js @@ -597,6 +597,10 @@ export class PackageAPI { */ "export"(symbols, arch, options) { var self = this; + if(this.sideEffects === false) { + this.setSideEffects(true); + Object.defineProperty(this, "sideEffects", { configurable: false, writable: false }); + } // Support `api.export("FooTest", {testOnly: true})` without // arch. From f146f2872059ab599c0a73ee8c3e1d33e8c3e5bf Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Wed, 16 Sep 2020 17:07:19 -0300 Subject: [PATCH 10/35] Don't stop analyzing files when first found, but accumulate the symbols required avoiding cycles. It was missing symbols on import scanner. --- tools/isobuild/bundler.js | 17 ++-- tools/isobuild/import-scanner.ts | 69 +++++++++---- tools/isobuild/js-analyze.js | 167 ++++++++++++++++++++----------- tools/isobuild/linker.js | 21 +++- 4 files changed, 180 insertions(+), 94 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 9ffe7ec1c31..757405b57d5 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1190,29 +1190,28 @@ class Target { const unibuild = sourceBatch.unibuild; const name = unibuild.pkg.name || null; - if(name === 'modules') { - debugger; - console.log(`sideEffects for: ${name} ${unibuild.pkg.sideEffects}`); - } // we will assume that every meteor package that exports something is a - // side effect true package - if(unibuild.pkg.sideEffects){ + // side effect true package, this is set on the package-api file when api.export is called + // console.log(`${unibuild.pkg.name} sideEffects: ${unibuild.pkg.sideEffects}`) + if(unibuild.pkg.sideEffects && name !== 'modules' || unibuild.arch.includes("legacy")){ return; } - if(name === 'modules'){ - // console.log(importMap); - } jsOutputFilesMap.get(name).files.forEach((file) => { const importedSymbolsFromFile = importMap.get(file.absPath) + if(file.absPath.includes("material-ui/styles/esm/jssPreset/jssPreset.js")){ + debugger; + } const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, file.hash, importedSymbolsFromFile, allFilesOnBundle, file.resolveMap || new Map()); + if(newSource) { file.dataString = newSource; file.data = Buffer.from(newSource, "utf8"); + file.source = Buffer.from(newSource, "utf8"); file.hash = sha1(file.data); } }) diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 00e1e780b0e..555433c7d5d 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -340,6 +340,7 @@ interface ImportIdentifiersEntry { interface File extends RawFile { type: string; + visitedFromRoots: [string]; sideEffects: boolean; sourcePath: string; targetPath?: string; @@ -348,6 +349,8 @@ interface File extends RawFile { deps?: Record; resolveMap?: Map; imports?: Record; + proxyImports?: Record; + exports?: [string]; lazy: boolean; bare?: boolean; // TODO Improve the sourceMap type. @@ -379,7 +382,9 @@ type ImportInfo = { } type JSAnalyzeInfo = { dependencies?: Record; + proxyDependencies?: Record; identifiers?: Record; + exports?: [string]; } export default class ImportScanner { @@ -994,6 +999,10 @@ export default class ImportScanner { return IMPORT_SCANNER_CACHE.get(file.hash); } + if (file.absPath.includes("material-ui/styles/esm/jssPreset/index.js")) { + debugger; + } + const result = findImportedModuleIdentifiers( this.getDataString(file), file.hash, @@ -1086,12 +1095,6 @@ export default class ImportScanner { } private scanFile(file: File, forDynamicImport = false, parentImportSymbols?: ImportIdentifiersEntry) { - if (file.imported === "static") { - // If we've already scanned this file non-dynamically, then we don't - // need to scan it again. - return; - } - if (forDynamicImport && file.imported === Status.DYNAMIC) { // If we've already scanned this file dynamically, then we don't @@ -1100,6 +1103,8 @@ export default class ImportScanner { } // Set file.imported to a truthy value (either "dynamic" or true). + //TODO: we need a better logic to stop scanning a file. + // if we first find a usage that gets it partially first, then we will include less than needed at the end setImportedStatus(file, forDynamicImport ? Status.DYNAMIC : Status.STATIC); if (file.reportPendingErrors && @@ -1112,12 +1117,13 @@ export default class ImportScanner { } try { - let importInfo; - if(!file.deps){ - importInfo = this.findImportedModuleIdentifiers(file, parentImportSymbols); - } + const importInfo = this.findImportedModuleIdentifiers(file, parentImportSymbols); + // this is actually an temporary dependency tree, we will need to analyze the + // children to see what they are exporting file.deps = file.deps || importInfo?.identifiers; file.imports = file.imports || importInfo?.dependencies; + file.proxyImports = file.proxyImports || importInfo?.proxyDependencies; + file.exports = file.exports || importInfo?.exports; } catch (e) { if (e.$ParseError) { (buildmessage as any).error(e.message, { @@ -1130,6 +1136,7 @@ export default class ImportScanner { throw e; } + each(file.deps, (info: ImportInfo, id: string) => { // Asynchronous module fetching only really makes sense in the // browser (even though it works equally well on the server), so @@ -1177,12 +1184,12 @@ export default class ImportScanner { // If depFile has already been scanned, this._scanFile will return // immediately thanks to the depFile.imported-checking logic at // the top of the method. - if(file.imports && file.imports[depFile.absPath]){ - this.scanFile(depFile, dynamic, file.imports[depFile.absPath]); - }else{ - this.scanFile(depFile, dynamic); - } + const parentImports = file.imports && file.imports[depFile.absPath]; + if(!(depFile.visitedFromRoots || []).includes(file.absPath)) { + depFile.visitedFromRoots = (depFile.visitedFromRoots || []).concat([file.absPath]); + this.scanFile(depFile, dynamic, parentImports); + } return; } @@ -1191,15 +1198,37 @@ export default class ImportScanner { return; } + const parentImports = file.imports && file.imports[depFile.absPath]; + + if(!(depFile.visitedFromRoots || []).includes(file.absPath)) { + depFile.visitedFromRoots = (depFile.visitedFromRoots || []).concat([file.absPath]); + this.scanFile(depFile, dynamic, parentImports); + } + + + // console.log(file.proxyImports); + + + if(file.proxyImports && + file.proxyImports[id] && + parentImportSymbols ){ + // if we are doing an wildcard export, it's time to make sure the file we included in the bundle + // should really be here + const isSomeSymbolImported = parentImportSymbols.deps.some((symbol) => depFile?.exports?.includes(symbol)) || parentImportSymbols.deps.includes("*"); + if(!isSomeSymbolImported){ + // console.log(parentImportSymbols) + // console.log(file.proxyImports) + // console.log("dropping file"); + return; + } + } + // Append this file to the output array and record its index. this.addFile(absImportedPath, depFile); - if(file.imports && file.imports[depFile.absPath]){ - this.scanFile(depFile, dynamic, file.imports[depFile.absPath]); - }else{ - this.scanFile(depFile, dynamic); - } + }); + } isWeb() { diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index 48822a98253..a720d55e196 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -62,6 +62,7 @@ export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, "require", "import", "export", + "exportDefault", "dynamicImport", "link", ]); @@ -72,7 +73,12 @@ export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, const ast = tryToParse(source, hash); importedIdentifierVisitor.visit(ast, source, possibleIndexes, parentImportSymbols, hasSideEffects); - return {identifiers: importedIdentifierVisitor.identifiers, dependencies: importedIdentifierVisitor.dependencies}; + return { + identifiers: importedIdentifierVisitor.identifiers, + dependencies: importedIdentifierVisitor.dependencies, + proxyDependencies: importedIdentifierVisitor.proxyDependencies, + exports: importedIdentifierVisitor.exports + }; } const importedIdentifierVisitor = new (class extends Visitor { @@ -82,6 +88,8 @@ const importedIdentifierVisitor = new (class extends Visitor { this.requireIsBound = false; this.identifiers = Object.create(null); this.dependencies = Object.create(null); + this.proxyDependencies = Object.create(null); + this.exports = []; // Defining this.possibleIndexes causes the Visitor to ignore any // subtrees of the AST that do not contain any indexes of identifiers @@ -121,10 +129,20 @@ const importedIdentifierVisitor = new (class extends Visitor { deps: [...new Set([...this.dependencies[id].deps, ...importIdentifiers])], sideEffects: this.dependencies[id].sideEffects || sideEffects, } : + {deps: importIdentifiers, sideEffects}; + } + + addProxyDependency(id, importIdentifiers, sideEffects) { + this.proxyDependencies[id] = this.proxyDependencies[id] ? { - deps: importIdentifiers, - sideEffects - }; + deps: [...new Set([...this.proxyDependencies[id].deps, ...importIdentifiers])], + sideEffects: this.proxyDependencies[id].sideEffects || sideEffects, + } : + {deps: importIdentifiers, sideEffects}; + } + + addExport(identifier) { + this.exports = this.exports.concat([identifier]); } visitFunctionExpression(path) { @@ -157,8 +175,31 @@ const importedIdentifierVisitor = new (class extends Visitor { const firstArg = args[0]; this.visitChildren(path); + const isModuleUsage = node.callee.type === "MemberExpression" && + // The Reify compiler sometimes renames references to the + // CommonJS module object for hygienic purposes, but it + // always does so by appending additional numbers. + isIdWithName(node.callee.object, /^module\d*$/); if (!isStringLiteral(firstArg)) { + // it can also be an export + if (isModuleUsage) { + const isDefaultExport = + isPropertyWithName(node.callee.property, "exportDefault"); + const isExport = + isPropertyWithName(node.callee.property, "export"); + + if (isDefaultExport) { + this.addExport('default'); + return; + } + if (isExport) { + firstArg.properties.forEach(({key}) => { + this.addExport(key.name); + }) + return; + } + } return; } @@ -169,63 +210,67 @@ const importedIdentifierVisitor = new (class extends Visitor { isIdWithName(node.callee, "import")) { this.addIdentifier(firstArg.value, "import", true); this.addDependency(firstArg.value, [], true); - } else if (node.callee.type === "MemberExpression" && - // The Reify compiler sometimes renames references to the - // CommonJS module object for hygienic purposes, but it - // always does so by appending additional numbers. - isIdWithName(node.callee.object, /^module\d*$/)) { - const propertyName = - isPropertyWithName(node.callee.property, "link") || - isPropertyWithName(node.callee.property, "dynamicImport"); - - if (propertyName) { - // if we have an object definition on module.link(), we are importing with ES6 possible without side effects - // otherwise we are considering it as a side effect import - if (args.length <= 1) { - this.addDependency(firstArg.value, [], true); - this.addIdentifier( - firstArg.value, - "import", - propertyName === "dynamicImport" - ); - return; - } - const secondArg = args[1]; - // if every prop is an string literal, we have something like: export { a, b, c } from './teste'; - if (secondArg.properties.every(({value}) => isStringLiteral(value)) && !this.hasSideEffects) { - // in this case, we need to verify the parent imports to make sure we follow the right tree path - const isImportedByParent = secondArg.properties.some(({value}) => { - if(this.parentImportSymbols){ - // console.log(this.parentImportSymbols); - // console.log(value.value); - } - return this.parentImportSymbols.deps.includes(value.value) || this.parentImportSymbols.deps.includes("*"); - }); - const isSomeSymbolImported = this.parentImportSymbols ? - this.parentImportSymbols.sideEffects || isImportedByParent - : true; - // console.log(isImportedByParent); - // console.log(this.parentImportSymbols); + } else { + if (isModuleUsage) { + const isImport = + isPropertyWithName(node.callee.property, "link") || + isPropertyWithName(node.callee.property, "dynamicImport"); + + if (isImport) { + // if we have an object definition on module.link(), we are importing with ES6 possible without side effects + // otherwise we are considering it as a side effect import + if (args.length <= 1) { + this.addDependency(firstArg.value, ['*'], true); + this.addIdentifier( + firstArg.value, + "import", + isImport === "dynamicImport" + ); + return; + } + const secondArg = args[1]; + // if every prop is an string literal, we have something like: export { a, b, c } from './teste'; + if (secondArg.properties.every(({value}) => isStringLiteral(value)) && !this.hasSideEffects) { + // in this case, we need to verify the parent imports to make sure we follow the right tree path + const isImportedByParent = secondArg.properties.some(({value}) => { + if (this.parentImportSymbols) { + // console.log(this.parentImportSymbols); + // console.log(value.value); + } + // if we are doing an wildcard export, export * from x.js, we will include this file to be analyzed later + // remember that this is actually temporary and this dependency can be dropped later in our analysis + if (value.value === "*") return true; + return this.parentImportSymbols.deps.includes(value.value) || this.parentImportSymbols.deps.includes("*"); + }); + const isSomeSymbolImported = this.parentImportSymbols ? + this.parentImportSymbols.sideEffects || isImportedByParent + : true; if (isSomeSymbolImported) { this.addIdentifier( firstArg.value, "import", - propertyName === "dynamicImport" + isImport === "dynamicImport" ); } - this.addDependency(firstArg.value, secondArg.properties.map(({key, value}) => this.parentImportSymbols.deps.includes(value.value) && key.name).filter(Boolean), false); - console.log(firstArg.value); - return; - } - this.addIdentifier( - firstArg.value, - "import", - propertyName === "dynamicImport" - ); - this.addDependency(firstArg.value, secondArg.properties.map(({key}) => key.name), false); + this.addDependency(firstArg.value, secondArg.properties.map(({key, value}) => { + const isImportedByParent = this.parentImportSymbols.deps.includes(value.value) || value.value === "*"; + return isImportedByParent && key.name; + }).filter(Boolean), false); + this.addProxyDependency(firstArg.value, secondArg.properties.map(({key}) => key.name || "*", false)); + return; + } + this.addIdentifier( + firstArg.value, + "import", + isImport === "dynamicImport" + ); + const importIdentifiers = secondArg.properties.map(({key}) => key.name || "*"); + // console.log(importIdentifiers); + this.addDependency(firstArg.value, importIdentifiers, false); + } } } } @@ -262,7 +307,7 @@ export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = "export", "exportDefault", "link", - "importDynamic" + "dynamicImport" ]); if (possibleIndexes.length === 0) { @@ -322,7 +367,6 @@ const removeUnusedExportsVisitor = new (class extends Visitor { visitCallExpression(path) { const node = path.getValue(); - const args = node.arguments; const firstArg = node.arguments[0]; this.visitChildren(path); @@ -332,25 +376,26 @@ const removeUnusedExportsVisitor = new (class extends Visitor { // CommonJS module object for hygienic purposes, but it // always does so by appending additional numbers. isIdWithName(node.callee.object, /^module\d*$/)) { + const isDefaultExport = + isPropertyWithName(node.callee.property, "defaultExport"); const isExport = - isPropertyWithName(node.callee.property, "export") || - isPropertyWithName(node.callee.property, "exportDefault"); + isPropertyWithName(node.callee.property, "export"); const isImport = isPropertyWithName(node.callee.property, "link") || isPropertyWithName(node.callee.property, "dynamicImport"); - if (isExport) { + if (isExport || isDefaultExport) { // if we have an object definition on module.export() if (firstArg) { - this.removeIdentifiers(path, firstArg, isPropertyWithName(node.callee.property, "exportDefault")); + this.removeIdentifiers(path, firstArg, isDefaultExport); } } - if(isImport){ + if (isImport) { const absPath = this.resolveMap.get(firstArg.value); const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; - - if(!fileIsInBundle) { + if (!fileIsInBundle) { + console.log(`removing file ${absPath}`) path.replace({ type: "BooleanLiteral", value: false, diff --git a/tools/isobuild/linker.js b/tools/isobuild/linker.js index 2450ca09f79..d7fd3c8f21b 100644 --- a/tools/isobuild/linker.js +++ b/tools/isobuild/linker.js @@ -267,10 +267,10 @@ _.extend(Module.prototype, { const tree = getTree(file); - if (file.aliasId) { - addToTree(file.aliasId, file.absModuleId, tree); - return; - } + // if (file.aliasId) { + // addToTree(file.aliasId, file.absModuleId, tree); + // return; + // } if (file.isDynamic()) { const servePath = files.pathJoin("dynamic", file.absModuleId); @@ -363,6 +363,9 @@ _.extend(Module.prototype, { chunks.push("{"); const keys = _.keys(t); _.each(keys, (key, i) => { + if(key === 'index.js'){ + console.log("indexxxx"); + } chunks.push(JSON.stringify(key), ":"); walk(t[key]); if (i < keys.length - 1) { @@ -746,6 +749,9 @@ const getPrelinkedOutputCached = require("optimism").wrap( map: file.sourceMap || null, }; + if(file.servePath.includes("createGenerateClassName/index.js")){ + console.log(file.source); + } var chunks = []; var pathNoSlash = convertColons(file.servePath.replace(/^\//, "")); @@ -1191,6 +1197,10 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, { } if (file.sourceMap) { + if(file.absModuleId && file.absModuleId.includes("createGenerateClassName")){ + console.log("INCLUDE"); + } + var sourceMap = file.sourceMap; sourceMap.mappings = headerContent + sourceMap.mappings; return { @@ -1200,6 +1210,9 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, { sourceMap: sourceMap }; } else { + if(file.absModuleId && file.absModuleId.includes("createGenerateClassName")){ + console.log("INCLUDE"); + } return { source: header + file.source + footer, sourcePath: file.sourcePath, From 90ecfaaefa977ba80b887a0cf0a902c9bea55f8b Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Wed, 23 Sep 2020 18:35:55 -0300 Subject: [PATCH 11/35] Remove entires sub trees instead of only one node. Also store globally the info of imports/exports/deps for later reconciliation. --- packages/babel-compiler/babel.js | 3 +- .../.npm/package/npm-shrinkwrap.json | 6 +- packages/minifier-js/minifier.js | 14 +- packages/minifier-js/package.js | 2 +- .../non-core/bundle-visualizer/package.js | 1 + tools/isobuild/bundler.js | 13 +- tools/isobuild/compiler-plugin.js | 15 +- tools/isobuild/import-scanner.ts | 269 +++++++++++++----- tools/isobuild/js-analyze.js | 75 +++-- tools/isobuild/linker.js | 3 - tools/isobuild/package-source.js | 4 +- tools/packaging/catalog/catalog-local.js | 2 +- 12 files changed, 284 insertions(+), 123 deletions(-) diff --git a/packages/babel-compiler/babel.js b/packages/babel-compiler/babel.js index 44e5d44337f..a2eb6462662 100644 --- a/packages/babel-compiler/babel.js +++ b/packages/babel-compiler/babel.js @@ -52,8 +52,7 @@ Babel = { try { const globalDefsKeys = Object.keys(globalDefsMapping); return Npm.require("@babel/core").transformSync(source, { - ast: false, - compact: false, + ...getMeteorBabel().getMinifierOptions(), plugins: [ function replaceStateVars({types: t}) { return { diff --git a/packages/minifier-js/.npm/package/npm-shrinkwrap.json b/packages/minifier-js/.npm/package/npm-shrinkwrap.json index 712e9dd22d5..95e49ea7714 100644 --- a/packages/minifier-js/.npm/package/npm-shrinkwrap.json +++ b/packages/minifier-js/.npm/package/npm-shrinkwrap.json @@ -22,9 +22,9 @@ "integrity": "sha512-Wonm7zOCIJzBGQdB+thsPar0kYuCIzYvxZwlBa87yi/Mdjv7Tip2cyVbLj5o0cFPN4EVkuTwb3GDDyUx2DGnGw==" }, "terser": { - "version": "4.7.0", - "resolved": "https://registry.npmjs.org/terser/-/terser-4.7.0.tgz", - "integrity": "sha512-Lfb0RiZcjRDXCC3OSHJpEkxJ9Qeqs6mp2v4jf2MHfy8vGERmVDuvjXdd/EnP5Deme5F2yBRBymKmKHCBg2echw==" + "version": "5.3.2", + "resolved": "https://registry.npmjs.org/terser/-/terser-5.3.2.tgz", + "integrity": "sha512-H67sydwBz5jCUA32ZRL319ULu+Su1cAoZnnc+lXnenGRYWyLE3Scgkt8mNoAsMx0h5kdo758zdoS0LG9rYZXDQ==" } } } diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index 39597eb7128..ec67df58513 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -48,8 +48,10 @@ meteorJsMinify = function (source, options) { }, {}); globalDefsMapping.Meteor = {...globalDefsMapping.Meteor, ...customSettings}; try { - var optimizedCode = Babel.replaceMeteorInternalState(source, globalDefsMapping) - var terserResult = terser.minify(optimizedCode, { + //TODO: uncomment this code + // var optimizedCode = Babel.replaceMeteorInternalState(source, globalDefsMapping) + console.log("Starting minify proccess"); + var terserResult = Meteor.wrapAsync(callback => terser.minify(source, { compress: { drop_debugger: false, unused: true, @@ -60,12 +62,11 @@ meteorJsMinify = function (source, options) { }, // passes: 2 }, - // mangle: {toplevel: true}, // Fix issue #9866, as explained in this comment: // https://github.com/mishoo/UglifyJS2/issues/1753#issuecomment-324814782 // And fix terser issue #117: https://github.com/terser-js/terser/issues/117 safari10: true, - }); + }).then((result) => callback(null, result)).catch((err) => callback(err, null)))(); if (typeof terserResult.code === "string") { result.code = terserResult.code; result.minifier = 'terser'; @@ -74,6 +75,11 @@ meteorJsMinify = function (source, options) { new Error("unknown terser.minify failure"); } } catch (e) { + console.log(e); + Npm.require("fs").writeFile('helloworld.js', source, function (err) { + if (err) return console.log(err); + console.log('Hello World > helloworld.txt'); + }); // Although Babel.minify can handle a wider variety of ECMAScript // 2015+ syntax, it is substantially slower than UglifyJS/terser, so // we use it only as a fallback. diff --git a/packages/minifier-js/package.js b/packages/minifier-js/package.js index b33b80d0aba..2ebd17de09f 100644 --- a/packages/minifier-js/package.js +++ b/packages/minifier-js/package.js @@ -4,7 +4,7 @@ Package.describe({ }); Npm.depends({ - terser: "4.7.0" + terser: "5.3.2" }); Package.onUse(function (api) { diff --git a/packages/non-core/bundle-visualizer/package.js b/packages/non-core/bundle-visualizer/package.js index fb625cc06fd..59d4d3adb5d 100644 --- a/packages/non-core/bundle-visualizer/package.js +++ b/packages/non-core/bundle-visualizer/package.js @@ -22,5 +22,6 @@ Package.onUse(function(api) { 'webapp', ]); api.mainModule('server.js', 'server'); + api.setSideEffects(true); api.mainModule('client.js', 'client'); }); diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 757405b57d5..18c7abca84a 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1136,7 +1136,7 @@ class Target { const isWeb = archinfo.matches(this.arch, 'web'); const isOs = archinfo.matches(this.arch, 'os'); - const jsOutputFilesMap = compilerPluginModule.PackageSourceBatch + const { jsOutputFilesMap, resolveMap } = compilerPluginModule.PackageSourceBatch .computeJsOutputFilesMap(sourceBatches); sourceBatches.forEach(batch => { @@ -1199,14 +1199,17 @@ class Target { jsOutputFilesMap.get(name).files.forEach((file) => { const importedSymbolsFromFile = importMap.get(file.absPath) - if(file.absPath.includes("material-ui/styles/esm/jssPreset/jssPreset.js")){ - debugger; + + // if it was commonjsImported we have nothing to do here + if(file.deps?.commonJsImported){ + return; } + if(file.absPath.includes("runtime/helpers/esm/arrayWithoutHoles.js")) debugger; const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, file.hash, importedSymbolsFromFile, allFilesOnBundle, - file.resolveMap || new Map()); + resolveMap.get(file.absPath) || new Map()); if(newSource) { file.dataString = newSource; @@ -3227,7 +3230,6 @@ function bundle({ }, function () { var packageSource = new PackageSource; packageSource.initFromAppDir(projectContext, exports.ignoreFiles); - var makeClientTarget = Profile( "bundler.bundle..makeClientTarget", function (app, webArch, options) { var client = new ClientTarget({ @@ -3280,6 +3282,7 @@ function bundle({ isopackCache: projectContext.isopackCache, includeCordovaUnibuild: projectContext.platformList.usesCordova() }); + console.log(app); const mergeAppWatchSets = () => { var projectAndLocalPackagesWatchSet = diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index 21f81a018fe..7c01c6aacce 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -1256,6 +1256,7 @@ export class PackageSourceBatch { // Returns a map from package names to arrays of JS output files. static computeJsOutputFilesMap(sourceBatches) { const map = new Map; + const resolveMap = new Map(); sourceBatches.forEach(batch => { const name = batch.unibuild.pkg.name || null; @@ -1354,6 +1355,7 @@ export class PackageSourceBatch { const scanner = new ImportScanner({ name, bundleArch: batch.processor.arch, + sideEffects: batch.unibuild.pkg.sideEffects, extensions: batch.importExtensions, sourceRoot: batch.sourceRoot, nodeModulesPaths, @@ -1443,6 +1445,8 @@ export class PackageSourceBatch { scannerMap.forEach((scanner, name) => { const isApp = ! name; const outputFiles = scanner.getOutputFiles(); + console.log(outputFiles.map(({absPath}) => absPath).includes("/Users/renancamargodecastro/meteor-projects/material-ui-teste/node_modules/react-is/index.js")); + const entry = map.get(name); if (entry.batch.useMeteorInstall) { @@ -1493,14 +1497,17 @@ export class PackageSourceBatch { entry.files = appFilesWithoutNodeModules; } else { + entry.files = outputFiles; } + scanner.resolveMap.forEach(function(value, key) { + resolveMap.set(key, value); + }); }); - - return this._watchOutputFiles(map); + return this._watchOutputFiles(map, resolveMap); } - static _watchOutputFiles(jsOutputFilesMap) { + static _watchOutputFiles(jsOutputFilesMap, resolveMap) { // Watch all output files produced by computeJsOutputFilesMap. jsOutputFilesMap.forEach(entry => { entry.files.forEach(file => { @@ -1527,7 +1534,7 @@ export class PackageSourceBatch { } }); }); - return jsOutputFilesMap; + return { jsOutputFilesMap, resolveMap }; } static _warnAboutMissingModules(missingModules) { diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 555433c7d5d..06df49bb51e 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -318,6 +318,7 @@ const useNodeStub: Omit = function () { export type ImportScannerOptions = { name: string; + sideEffects: boolean; bundleArch: string; extensions: string[]; sourceRoot: string; @@ -340,17 +341,15 @@ interface ImportIdentifiersEntry { interface File extends RawFile { type: string; - visitedFromRoots: [string]; sideEffects: boolean; sourcePath: string; targetPath?: string; servePath?: string; absModuleId?: string; deps?: Record; - resolveMap?: Map; imports?: Record; proxyImports?: Record; - exports?: [string]; + exports: [string]; lazy: boolean; bare?: boolean; // TODO Improve the sourceMap type. @@ -374,6 +373,8 @@ type MissingMap = Record; type ImportInfo = { dynamic: boolean; possiblySpurious: boolean; + commonJsImported: boolean; + sideEffects: boolean; parentPath: string; bundleArch?: string; packageName?: string; @@ -382,17 +383,26 @@ type ImportInfo = { } type JSAnalyzeInfo = { dependencies?: Record; - proxyDependencies?: Record; + proxyDependencies?: Record>; identifiers?: Record; exports?: [string]; } +type ImportTreeNode = { + children?: [ImportTreeNode?]; + absImportedPath?: string; + depFile?: File; +} export default class ImportScanner { public name: string; private bundleArch: string; private sourceRoot: string; + private sideEffects: boolean; private nodeModulesPaths: string[]; + private visitedFromRoots: Record + private visitedFromRootsCache: Record + private resolveMap: Map> private defaultHandlers: DefaultHandlers; private resolver: Resolver; @@ -401,19 +411,76 @@ export default class ImportScanner { private realPathCache: Record = Object.create(null); private allMissingModules: MissingMap = Object.create(null); private outputFiles: File[] = []; + private filesInfo: Map; + + + private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo): any { + /* + dependencies?: Record; + proxyDependencies?: Record; + identifiers?: Record; + exports?: [string] + */ + file.deps = file.deps || {}; + Object.entries(importInfo.identifiers || {}).forEach(([key, value]) => { + if(!file.deps[key]) { + file.deps[key] = value; + return; + } + file.deps[key] = { + ...file.deps[key], + commonJsImported: value.commonJsImported || file.deps[key].commonJsImported, + sideEffects: value.sideEffects || file.deps[key].sideEffects + }; + }); + + file.imports = file.imports || {}; + Object.entries(importInfo.dependencies || {}).forEach(([key, value]) => { + if(file.imports && file.imports[key]){ + file.imports[key].deps = Array.from(new Set([...(file.imports[key].deps || []), ...(value.deps || [])])); + }else{ + file.imports[key] = value; + } + }); + + file.proxyImports = file.proxyImports || {}; + Object.entries(importInfo.proxyDependencies || {}).forEach(([key, value]) => { + if(file.proxyImports && file.proxyImports[key]){ + Object.entries(value).forEach(([importKey, importValue]) => { + if(!file.proxyImports[key][importKey]) { + file.proxyImports[key][importKey] = importValue; + } + }) + }else{ + file.proxyImports[key] = value; + } + }); + + file.exports = file.exports || []; + file.exports = file.exports.concat(importInfo.exports || []); + + return file; + } + constructor({ name, bundleArch, + sideEffects, extensions, sourceRoot, nodeModulesPaths = [], cacheDir, }: ImportScannerOptions) { this.name = name; + this.resolveMap = new Map(); + this.filesInfo = new Map(); + this.visitedFromRootsCache = {}; this.bundleArch = bundleArch; + this.sideEffects = sideEffects; this.sourceRoot = sourceRoot; this.nodeModulesPaths = nodeModulesPaths; + this.visitedFromRoots = {}; this.defaultHandlers = new DefaultHandlers({ cacheDir, @@ -751,10 +818,21 @@ export default class ImportScanner { } } + addImportTree(root : ImportTreeNode){ + if(root == null) return; + const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} + + this.addFile(root.absImportedPath, {...root.depFile, lazy:false, imported: true, ...importInfo}); + + for(let child of root.children || []){ + this.addImportTree(child); + } + } scanImports() { this.outputFiles.forEach(file => { if (! file.lazy) { - this.scanFile(file); + const importTree = this.scanFile(file, false, {deps:["*"]}); + this.addImportTree(importTree); } }); @@ -808,7 +886,7 @@ export default class ImportScanner { } if (staticImportInfo) { - this.scanFile({ + const tree = this.scanFile({ ...fakeStub, // By specifying the .deps property of this fake file ahead of // time, we can avoid calling findImportedModuleIdentifiers in @@ -816,13 +894,15 @@ export default class ImportScanner { // doesn't have a .data or .dataString property. deps: { [id]: staticImportInfo }, }, false); // !forDynamicImport + this.addImportTree(tree); } if (dynamicImportInfo) { - this.scanFile({ + const tree = this.scanFile({ ...fakeStub, deps: { [id]: dynamicImportInfo }, }, true); // forDynamicImport + this.addImportTree(tree); } }); @@ -993,23 +1073,24 @@ export default class ImportScanner { private findImportedModuleIdentifiers( file: File, - parentImportSymbols?: ImportIdentifiersEntry + parentImportSymbols?: ImportIdentifiersEntry, + isCommonJsImported: boolean = false, + hasSideEffects: boolean = false ): JSAnalyzeInfo { - if (IMPORT_SCANNER_CACHE.has(file.hash)) { - return IMPORT_SCANNER_CACHE.get(file.hash); - } + // if (IMPORT_SCANNER_CACHE.has(file.hash)) { + // return IMPORT_SCANNER_CACHE.get(file.hash); + // } - if (file.absPath.includes("material-ui/styles/esm/jssPreset/index.js")) { - debugger; - } const result = findImportedModuleIdentifiers( this.getDataString(file), file.hash, parentImportSymbols, - file.sideEffects + hasSideEffects, + isCommonJsImported ); + // there should always be file.hash, but better safe than sorry if (file.hash) { IMPORT_SCANNER_CACHE.set(file.hash, result); @@ -1067,14 +1148,6 @@ export default class ImportScanner { return file.alias; } } - if(resolved) { - const file = this.getFile(resolved.path); - if(!file) return resolved; - if (!file.resolveMap) { - file.resolveMap = new Map(); - } - file.resolveMap.set(id, resolved.path); - } return resolved; } @@ -1094,13 +1167,15 @@ export default class ImportScanner { return relativeId; } - private scanFile(file: File, forDynamicImport = false, parentImportSymbols?: ImportIdentifiersEntry) { - if (forDynamicImport && - file.imported === Status.DYNAMIC) { - // If we've already scanned this file dynamically, then we don't - // need to scan it dynamically again. - return; - } + private scanFile(file: File, + forDynamicImport: boolean = false, + parentImportSymbols?: ImportIdentifiersEntry, + isCommonJsImported: boolean = false, + treeHasSideEffects: boolean = false) : ImportTreeNode { + + if(file.absPath.includes("runtime/helpers/esm/arrayWithoutHoles.js")) debugger; + + const hasSideEffects = treeHasSideEffects || this.sideEffects || file.sideEffects; // Set file.imported to a truthy value (either "dynamic" or true). //TODO: we need a better logic to stop scanning a file. @@ -1113,17 +1188,30 @@ export default class ImportScanner { // Any errors reported to InputFile#error were saved but not // reported at compilation time. Now that we know the file has been // imported, it's time to report those errors. - return; + return { + children: [], + absImportedPath: file.absPath, + depFile: file, + }; } + let importInfo; + let mergedInfo; try { - const importInfo = this.findImportedModuleIdentifiers(file, parentImportSymbols); + importInfo = this.findImportedModuleIdentifiers( + file, + parentImportSymbols, + isCommonJsImported, + hasSideEffects, + ); // this is actually an temporary dependency tree, we will need to analyze the // children to see what they are exporting - file.deps = file.deps || importInfo?.identifiers; - file.imports = file.imports || importInfo?.dependencies; - file.proxyImports = file.proxyImports || importInfo?.proxyDependencies; - file.exports = file.exports || importInfo?.exports; + const cachedFileInfo = this.filesInfo.get(file.absPath) || {}; + mergedInfo = this.mergeImportInfo(cachedFileInfo, importInfo); + // we only need the local view here, later we will need the global, merged one + this.filesInfo.set(file.absPath, mergedInfo); + this.mergeImportInfo(file, importInfo) + } catch (e) { if (e.$ParseError) { (buildmessage as any).error(e.message, { @@ -1131,13 +1219,20 @@ export default class ImportScanner { line: e.loc.line, column: e.loc.column, }); - return; + return {}; } throw e; } + const rootChildren : ImportTreeNode = { + children: [], + absImportedPath: file.absPath, + depFile: file, + }; each(file.deps, (info: ImportInfo, id: string) => { + + let childrenDep : ImportTreeNode; // Asynchronous module fetching only really makes sense in the // browser (even though it works equally well on the server), so // it's better if forDynamicImport never becomes true on the server. @@ -1153,17 +1248,26 @@ export default class ImportScanner { } if(file.imports && file.imports[id]){ file.imports[absImportedPath] = file.imports[id]; - delete file.imports[id] + if(mergedInfo){ + mergedInfo.imports[absImportedPath] = mergedInfo.imports[id]; + this.filesInfo.set(file.absPath, mergedInfo) + } } - if(!file.resolveMap){ - file.resolveMap = new Map(); + if(absImportedPath) { + if (!this.resolveMap.get(file.absPath)) { + this.resolveMap.set(file.absPath, new Map()); + } + this.resolveMap.get(file.absPath)?.set(id, absImportedPath); } - file.resolveMap.set(id, absImportedPath); let depFile = this.getFile(absImportedPath); + const visitFrom = `${file.absPath}->${absImportedPath}` + + const depHasSideEffects = info.sideEffects || hasSideEffects; if (depFile) { + // We should never have stored a fake file in this.outputFiles, so // it's surprising if depFile[fakeSymbol] is true. assert.notStrictEqual(depFile[fakeSymbol], true); @@ -1181,14 +1285,22 @@ export default class ImportScanner { } } - // If depFile has already been scanned, this._scanFile will return - // immediately thanks to the depFile.imported-checking logic at - // the top of the method. - const parentImports = file.imports && file.imports[depFile.absPath]; - - if(!(depFile.visitedFromRoots || []).includes(file.absPath)) { - depFile.visitedFromRoots = (depFile.visitedFromRoots || []).concat([file.absPath]); - this.scanFile(depFile, dynamic, parentImports); + const parentImports = file.imports && file.imports[absImportedPath]; + + if(!(this.visitedFromRoots[absImportedPath] || []).includes(file.absPath)) { + if(!this.visitedFromRoots[absImportedPath]) this.visitedFromRoots[absImportedPath] = []; + this.visitedFromRoots[absImportedPath].push(file.absPath); + const importTreeNode = this.scanFile( + depFile, + dynamic, + parentImports, + isCommonJsImported || info.commonJsImported, + depHasSideEffects + ); + this.visitedFromRootsCache[visitFrom] = importTreeNode; + rootChildren.children?.push(importTreeNode); + }else{ + rootChildren.children?.push(this.visitedFromRootsCache[visitFrom]); } return; } @@ -1198,37 +1310,56 @@ export default class ImportScanner { return; } - const parentImports = file.imports && file.imports[depFile.absPath]; - - if(!(depFile.visitedFromRoots || []).includes(file.absPath)) { - depFile.visitedFromRoots = (depFile.visitedFromRoots || []).concat([file.absPath]); - this.scanFile(depFile, dynamic, parentImports); + const parentImports = file.imports && file.imports[absImportedPath]; + if(!(this.visitedFromRoots[absImportedPath] || []).includes(file.absPath)) { + if(!this.visitedFromRoots[absImportedPath]) this.visitedFromRoots[absImportedPath] = []; + this.visitedFromRoots[absImportedPath].push(file.absPath); + childrenDep = this.scanFile( + depFile, + dynamic, + parentImports, + isCommonJsImported || info.commonJsImported, + depHasSideEffects + ); + this.visitedFromRootsCache[visitFrom] = childrenDep; + }else{ + childrenDep = this.visitedFromRootsCache[visitFrom]; } - // console.log(file.proxyImports); - + const depRoot: ImportTreeNode = { + children: childrenDep?.children, + absImportedPath, + depFile, + }; if(file.proxyImports && file.proxyImports[id] && - parentImportSymbols ){ - // if we are doing an wildcard export, it's time to make sure the file we included in the bundle + parentImportSymbols && + !isCommonJsImported && + !info.commonJsImported && + !this.sideEffects && + !depHasSideEffects){ + // if we are doing an wildcard export, it's time to make sure the file we included in the bundle // should really be here - const isSomeSymbolImported = parentImportSymbols.deps.some((symbol) => depFile?.exports?.includes(symbol)) || parentImportSymbols.deps.includes("*"); - if(!isSomeSymbolImported){ - // console.log(parentImportSymbols) - // console.log(file.proxyImports) - // console.log("dropping file"); - return; + + const depFileImportInfo = depFile.exports || this.filesInfo.get(absImportedPath)?.exports; + // @ts-ignore + const isSomeSymbolImported = parentImportSymbols.deps.some((symbol) => { + // @ts-ignore + return depFileImportInfo?.includes(file.proxyImports[id][symbol]) || depFileImportInfo?.includes(symbol); + }) || parentImportSymbols.deps.includes("*"); + if(isSomeSymbolImported){ + rootChildren.children?.push(depRoot); } + }else { + // Append this file to the output array and record its index. + rootChildren.children?.push(depRoot); } - - // Append this file to the output array and record its index. - this.addFile(absImportedPath, depFile); - - }); + this.visitedFromRootsCache[file.absPath] = rootChildren; + return rootChildren; } isWeb() { diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index a720d55e196..8eb591fd42b 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -1,4 +1,5 @@ -import {parse} from 'meteor-babel'; +import {parse, getDefaultOptions} from 'meteor-babel'; +import generate from '@babel/generator'; import {analyze as analyzeScope} from 'escope'; import LRU from "lru-cache"; @@ -57,7 +58,7 @@ function tryToParse(source, hash) { * if the tokens were actually what we thought they were (a `require` * function call, or an `import` or `export` statement). */ -export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, hasSideEffects) { +export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, hasSideEffects, isCommonJsImported) { const possibleIndexes = findPossibleIndexes(source, [ "require", "import", @@ -72,7 +73,7 @@ export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, } const ast = tryToParse(source, hash); - importedIdentifierVisitor.visit(ast, source, possibleIndexes, parentImportSymbols, hasSideEffects); + importedIdentifierVisitor.visit(ast, source, possibleIndexes, parentImportSymbols, hasSideEffects, isCommonJsImported); return { identifiers: importedIdentifierVisitor.identifiers, dependencies: importedIdentifierVisitor.dependencies, @@ -82,10 +83,11 @@ export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, } const importedIdentifierVisitor = new (class extends Visitor { - reset(rootPath, code, possibleIndexes, parentImportSymbols, hasSideEffects = false) { + reset(rootPath, code, possibleIndexes, parentImportSymbols, hasSideEffects = false, isCommonJsImported = false) { this.parentImportSymbols = parentImportSymbols || []; this.hasSideEffects = hasSideEffects; this.requireIsBound = false; + this.isCommonJsImported = false; this.identifiers = Object.create(null); this.dependencies = Object.create(null); this.proxyDependencies = Object.create(null); @@ -99,7 +101,7 @@ const importedIdentifierVisitor = new (class extends Visitor { this.possibleIndexes = possibleIndexes; } - addIdentifier(id, type, dynamic) { + addIdentifier(id, type, dynamic, commonJsImported = false, sideEffects = false) { const entry = hasOwn.call(this.identifiers, id) ? this.identifiers[id] : this.identifiers[id] = { @@ -121,6 +123,8 @@ const importedIdentifierVisitor = new (class extends Visitor { // registered by import statements should be trusted absolutely. entry.possiblySpurious = false; } + entry.commonJsImported = entry.commonJsImported || commonJsImported; + entry.sideEffects = entry.sideEffects || sideEffects; } addDependency(id, importIdentifiers, sideEffects) { @@ -132,13 +136,15 @@ const importedIdentifierVisitor = new (class extends Visitor { {deps: importIdentifiers, sideEffects}; } - addProxyDependency(id, importIdentifiers, sideEffects) { - this.proxyDependencies[id] = this.proxyDependencies[id] ? - { - deps: [...new Set([...this.proxyDependencies[id].deps, ...importIdentifiers])], - sideEffects: this.proxyDependencies[id].sideEffects || sideEffects, - } : - {deps: importIdentifiers, sideEffects}; + addProxyDependency(id, map) { + if (this.proxyDependencies[id]) { + const oldDeps = this.proxyDependencies[id]; + map.forEach(([key, value]) => { + oldDeps[key] = value; + }) + } else { + this.proxyDependencies[id] = Object.fromEntries(map); + } } addExport(identifier) { @@ -204,12 +210,12 @@ const importedIdentifierVisitor = new (class extends Visitor { } if (isIdWithName(node.callee, "require")) { - this.addIdentifier(firstArg.value, "require"); - this.addDependency(firstArg.value, [], true); + this.addIdentifier(firstArg.value, "require", false, true, true); + this.addDependency(firstArg.value, ['*'], true); } else if (node.callee.type === "Import" || isIdWithName(node.callee, "import")) { - this.addIdentifier(firstArg.value, "import", true); - this.addDependency(firstArg.value, [], true); + this.addIdentifier(firstArg.value, "import", true, false, true); + this.addDependency(firstArg.value, ['*'], true); } else { if (isModuleUsage) { const isImport = @@ -231,6 +237,9 @@ const importedIdentifierVisitor = new (class extends Visitor { const secondArg = args[1]; // if every prop is an string literal, we have something like: export { a, b, c } from './teste'; if (secondArg.properties.every(({value}) => isStringLiteral(value)) && !this.hasSideEffects) { + if((this.parentImportSymbols.deps || []).includes("createUnarySpacing")){ + // debugger; + } // in this case, we need to verify the parent imports to make sure we follow the right tree path const isImportedByParent = secondArg.properties.some(({value}) => { if (this.parentImportSymbols) { @@ -240,6 +249,9 @@ const importedIdentifierVisitor = new (class extends Visitor { // if we are doing an wildcard export, export * from x.js, we will include this file to be analyzed later // remember that this is actually temporary and this dependency can be dropped later in our analysis if (value.value === "*") return true; + if(!this.parentImportSymbols.deps){ + debugger; + } return this.parentImportSymbols.deps.includes(value.value) || this.parentImportSymbols.deps.includes("*"); }); const isSomeSymbolImported = this.parentImportSymbols ? @@ -247,18 +259,21 @@ const importedIdentifierVisitor = new (class extends Visitor { : true; - if (isSomeSymbolImported) { - this.addIdentifier( - firstArg.value, - "import", - isImport === "dynamicImport" - ); - } + if (isSomeSymbolImported || this.isCommonJsImported) { + this.addIdentifier( + firstArg.value, + "import", + isImport === "dynamicImport" + ); + } this.addDependency(firstArg.value, secondArg.properties.map(({key, value}) => { - const isImportedByParent = this.parentImportSymbols.deps.includes(value.value) || value.value === "*"; - return isImportedByParent && key.name; + const isImportedByParent = this.parentImportSymbols.deps.includes(value.value) || this.parentImportSymbols.deps.includes("*") || value.value === "*"; + return isImportedByParent && (key.name || value.value); }).filter(Boolean), false); - this.addProxyDependency(firstArg.value, secondArg.properties.map(({key}) => key.name || "*", false)); + + // key and value are inverted by purpose + this.addProxyDependency(firstArg.value, secondArg.properties.map(({key, value}) => [value.value, key.name] || ["*", "*"])); + secondArg.properties.forEach(({key, value}) => this.addExport(value.value || key.name || "*")); return; } this.addIdentifier( @@ -316,7 +331,7 @@ export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = const ast = tryToParse(source, hash); removeUnusedExportsVisitor.visit(ast, source, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap); - const newSource = require('@babel/generator').default(ast).code; + const newSource = generate(ast, getDefaultOptions()).code; return {source: newSource, madeChanges: removeUnusedExportsVisitor.madeChanges}; } @@ -356,7 +371,8 @@ const removeUnusedExportsVisitor = new (class extends Visitor { } const exportKey = property.key.value || property.key.name; - let returnValue = (this.exportInfo?.deps || []).includes(exportKey) ? property : null; + const exportInfoSafe = this.exportInfo?.deps || []; + let returnValue = exportInfoSafe.includes(exportKey) || exportInfoSafe.includes("*") ? property : null; if (!returnValue) { console.log(`Removing ${exportKey}`) } @@ -392,10 +408,11 @@ const removeUnusedExportsVisitor = new (class extends Visitor { } } if (isImport) { + if(firstArg.value && firstArg.value.startsWith("meteor/")) return; + if(node.arguments.length <= 1) return; const absPath = this.resolveMap.get(firstArg.value); const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; if (!fileIsInBundle) { - console.log(`removing file ${absPath}`) path.replace({ type: "BooleanLiteral", value: false, diff --git a/tools/isobuild/linker.js b/tools/isobuild/linker.js index d7fd3c8f21b..67694413405 100644 --- a/tools/isobuild/linker.js +++ b/tools/isobuild/linker.js @@ -363,9 +363,6 @@ _.extend(Module.prototype, { chunks.push("{"); const keys = _.keys(t); _.each(keys, (key, i) => { - if(key === 'index.js'){ - console.log("indexxxx"); - } chunks.push(JSON.stringify(key), ":"); walk(t[key]); if (i < keys.length - 1) { diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js index d8b82d5d2a4..2df0d19e701 100644 --- a/tools/isobuild/package-source.js +++ b/tools/isobuild/package-source.js @@ -897,6 +897,8 @@ _.extend(PackageSource.prototype, { self.name = null; self.sourceRoot = appDir; self.serveRoot = '/'; + const sideEffects = JSON.parse(files.readFile(projectContext.meteorConfig.packageJsonPath)).sideEffects || false; + self.sideEffects = sideEffects; // Determine used packages. Note that these are the same for all arches, // because there's no way to specify otherwise in .meteor/packages. @@ -936,8 +938,6 @@ _.extend(PackageSource.prototype, { const nodeModulesToRecompile = projectContext.meteorConfig .getNodeModulesToRecompile(arch, nodeModulesToRecompileByArch); - const sideEffects = JSON.parse(files.readFile(projectContext.meteorConfig.packageJsonPath)).sideEffects || false; - console.log(`Reading side effects: ${sideEffects}`); // XXX what about /web.browser/* etc, these directories could also // be for specific client targets. diff --git a/tools/packaging/catalog/catalog-local.js b/tools/packaging/catalog/catalog-local.js index 827ee6e950d..3c7a1c9e0fd 100644 --- a/tools/packaging/catalog/catalog-local.js +++ b/tools/packaging/catalog/catalog-local.js @@ -418,7 +418,7 @@ _.extend(LocalCatalog.prototype, { var self = this; if (! _.has(self.packages, name)) return null; - if(name === 'base64'){ + if(!name){ console.log(`getPackageSource -> ${self.packages[name].packageSource.sideEffects}`); } return self.packages[name].packageSource; From 9d6f99d3f07aaaf094a85c6630c3e950968c2fd8 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 24 Sep 2020 10:51:58 -0300 Subject: [PATCH 12/35] When adding the import tree to the files, do not run over the same node, mark it as visited/unvisited --- tools/isobuild/bundler.js | 1 - tools/isobuild/import-scanner.ts | 23 +++++++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 18c7abca84a..204e909dd0d 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1204,7 +1204,6 @@ class Target { if(file.deps?.commonJsImported){ return; } - if(file.absPath.includes("runtime/helpers/esm/arrayWithoutHoles.js")) debugger; const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, file.hash, importedSymbolsFromFile, diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 06df49bb51e..2e066983c63 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -387,10 +387,16 @@ type JSAnalyzeInfo = { identifiers?: Record; exports?: [string]; } +enum ImportTreeNodeStatus { + WHITE, + GRAY, + BLACK +} type ImportTreeNode = { children?: [ImportTreeNode?]; absImportedPath?: string; depFile?: File; + status?: ImportTreeNodeStatus; } export default class ImportScanner { @@ -818,15 +824,21 @@ export default class ImportScanner { } } - addImportTree(root : ImportTreeNode){ - if(root == null) return; + addImportTree(root? : ImportTreeNode){ + if(!root) return; + if(!root.status) root.status = ImportTreeNodeStatus.WHITE; const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} - - this.addFile(root.absImportedPath, {...root.depFile, lazy:false, imported: true, ...importInfo}); + if(root.absImportedPath) { + this.addFile(root.absImportedPath, { ...root.depFile, lazy: false, imported: true, ...importInfo }); + } + root.status = ImportTreeNodeStatus.GRAY; for(let child of root.children || []){ - this.addImportTree(child); + if(!child?.status) { + this.addImportTree(child); + } } + root.status = ImportTreeNodeStatus.BLACK; } scanImports() { this.outputFiles.forEach(file => { @@ -1173,7 +1185,6 @@ export default class ImportScanner { isCommonJsImported: boolean = false, treeHasSideEffects: boolean = false) : ImportTreeNode { - if(file.absPath.includes("runtime/helpers/esm/arrayWithoutHoles.js")) debugger; const hasSideEffects = treeHasSideEffects || this.sideEffects || file.sideEffects; From 0aa9d4a43f51282a1e45a71a5e98d8805d6e048f Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 24 Sep 2020 15:01:43 -0300 Subject: [PATCH 13/35] Fix dynamic imports not working. --- tools/isobuild/import-scanner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 2e066983c63..a6536f6d598 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -829,7 +829,7 @@ export default class ImportScanner { if(!root.status) root.status = ImportTreeNodeStatus.WHITE; const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} if(root.absImportedPath) { - this.addFile(root.absImportedPath, { ...root.depFile, lazy: false, imported: true, ...importInfo }); + this.addFile(root.absImportedPath, { ...root.depFile, ...importInfo }); } root.status = ImportTreeNodeStatus.GRAY; From 4e719f6ec6e0383a5a5afee2559bbc19c22f64b3 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 29 Sep 2020 11:16:52 -0300 Subject: [PATCH 14/35] Do not remove imports from node native modules if we are on the server bundle. Remembering that those packages are not bundled in the server build, so we need to verify them explicitly --- tools/isobuild/bundler.js | 7 +- tools/isobuild/compiler-plugin.js | 7 +- tools/isobuild/import-scanner.ts | 40 ++++++--- tools/isobuild/js-analyze.js | 22 +++-- tools/isobuild/linker.js | 2 + tools/isobuild/package-source.js | 6 +- tools/tests/apps/app-config/.meteor/versions | 89 +++++++++---------- tools/tests/apps/app-config/package-lock.json | 8 +- 8 files changed, 105 insertions(+), 76 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 204e909dd0d..12975d1bca9 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1204,11 +1204,14 @@ class Target { if(file.deps?.commonJsImported){ return; } - const { source:newSource, madeChanges } = removeUnusedExports(file.dataString, + const { source:newSource, madeChanges } = removeUnusedExports( + file.dataString, file.hash, importedSymbolsFromFile, allFilesOnBundle, - resolveMap.get(file.absPath) || new Map()); + resolveMap.get(file.absPath) || new Map(), + unibuild.arch + ); if(newSource) { file.dataString = newSource; diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index 7c01c6aacce..f429e59c305 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -1501,7 +1501,11 @@ export class PackageSourceBatch { entry.files = outputFiles; } scanner.resolveMap.forEach(function(value, key) { - resolveMap.set(key, value); + const current = resolveMap.get(key) || new Map(); + for(const [newKey, newValue] of value.entries()){ + current.set(newKey, newValue); + } + resolveMap.set(key, current); }); }); return this._watchOutputFiles(map, resolveMap); @@ -1766,6 +1770,7 @@ export class PackageSourceBatch { const ret = linkedFiles.map((file) => { const sm = (typeof file.sourceMap === 'string') ? JSON.parse(file.sourceMap) : file.sourceMap; + return { type: "js", // This is a string... but we will convert it to a Buffer diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index a6536f6d598..f17a3a2f8d7 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -339,9 +339,16 @@ interface ImportIdentifiersEntry { sideEffects: boolean; } +interface AnalyzeInfo { + deps?: Record; + imports?: Record; + proxyImports?: Record; + exports: [string]; +} + interface File extends RawFile { type: string; - sideEffects: boolean; + sideEffects?: boolean; sourcePath: string; targetPath?: string; servePath?: string; @@ -349,7 +356,7 @@ interface File extends RawFile { deps?: Record; imports?: Record; proxyImports?: Record; - exports: [string]; + exports?: [string?]; lazy: boolean; bare?: boolean; // TODO Improve the sourceMap type. @@ -420,7 +427,7 @@ export default class ImportScanner { private filesInfo: Map; - private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo): any { + private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo): AnalyzeInfo { /* dependencies?: Record; proxyDependencies?: Record; @@ -746,9 +753,11 @@ export default class ImportScanner { sourceFile.hash = sha1(sourceFile.data); sourceFile.deps = sourceFile.deps || Object.create(null); sourceFile.deps![relativeId] = { + commonJsImported: false, + sideEffects: false, dynamic: false, possiblySpurious: false, - parentPath: absSourcePath, + parentPath: absSourcePath }; } } @@ -829,7 +838,7 @@ export default class ImportScanner { if(!root.status) root.status = ImportTreeNodeStatus.WHITE; const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} if(root.absImportedPath) { - this.addFile(root.absImportedPath, { ...root.depFile, ...importInfo }); + this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo)); } root.status = ImportTreeNodeStatus.GRAY; @@ -843,7 +852,7 @@ export default class ImportScanner { scanImports() { this.outputFiles.forEach(file => { if (! file.lazy) { - const importTree = this.scanFile(file, false, {deps:["*"]}); + const importTree = this.scanFile(file, false, {deps:["*"], sideEffects: this.sideEffects}); this.addImportTree(importTree); } }); @@ -899,12 +908,12 @@ export default class ImportScanner { if (staticImportInfo) { const tree = this.scanFile({ - ...fakeStub, + ...fakeStub, exports: ["*"], sideEffects: this.sideEffects, // By specifying the .deps property of this fake file ahead of // time, we can avoid calling findImportedModuleIdentifiers in // the _scanFile method, which is important because this file // doesn't have a .data or .dataString property. - deps: { [id]: staticImportInfo }, + deps: { [id]: staticImportInfo } }, false); // !forDynamicImport this.addImportTree(tree); } @@ -912,6 +921,7 @@ export default class ImportScanner { if (dynamicImportInfo) { const tree = this.scanFile({ ...fakeStub, + exports: ["*"], sideEffects: this.sideEffects, deps: { [id]: dynamicImportInfo }, }, true); // forDynamicImport this.addImportTree(tree); @@ -1207,7 +1217,7 @@ export default class ImportScanner { } let importInfo; - let mergedInfo; + let mergedInfo : AnalyzeInfo; try { importInfo = this.findImportedModuleIdentifiers( file, @@ -1241,6 +1251,7 @@ export default class ImportScanner { depFile: file, }; + // @ts-ignore each(file.deps, (info: ImportInfo, id: string) => { let childrenDep : ImportTreeNode; @@ -1259,12 +1270,13 @@ export default class ImportScanner { } if(file.imports && file.imports[id]){ file.imports[absImportedPath] = file.imports[id]; - if(mergedInfo){ + if(mergedInfo && mergedInfo.imports){ mergedInfo.imports[absImportedPath] = mergedInfo.imports[id]; this.filesInfo.set(file.absPath, mergedInfo) } } + if(absImportedPath) { if (!this.resolveMap.get(file.absPath)) { this.resolveMap.set(file.absPath, new Map()); @@ -1763,6 +1775,8 @@ export default class ImportScanner { } const info: ImportInfo = { + commonJsImported: false, + sideEffects: parentFile.sideEffects || false, packageName: this.name, parentPath: absParentPath, bundleArch: this.bundleArch, @@ -1773,7 +1787,7 @@ export default class ImportScanner { // if the parent module was imported dynamically, since that makes // this import effectively dynamic, even if the parent module // imported the given id with a static import or require. - parentWasDynamic: forDynamicImport, + parentWasDynamic: forDynamicImport }; if (parentFile && @@ -1941,6 +1955,8 @@ export default class ImportScanner { const relSourcePath = pathRelative(this.sourceRoot, source.path); this.addFile(source.path, { + exports: pkgFile.exports, + sideEffects: pkgFile.sideEffects, type: "js", alias, absPath: absPkgJsonPath, @@ -1952,7 +1968,7 @@ export default class ImportScanner { servePath: stripLeadingSlash(sourceAbsModuleId), lazy: true, imported: false, - implicit: true, + implicit: true }); } }); diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index 8eb591fd42b..aa0caeacabe 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -5,6 +5,8 @@ import LRU from "lru-cache"; import Visitor from "reify/lib/visitor.js"; import {findPossibleIndexes} from "reify/lib/utils.js"; +import {matches as archMatches} from "../utils/archinfo"; +import Resolver from "./resolver"; const hasOwn = Object.prototype.hasOwnProperty; const objToStr = Object.prototype.toString @@ -84,7 +86,7 @@ export function findImportedModuleIdentifiers(source, hash, parentImportSymbols, const importedIdentifierVisitor = new (class extends Visitor { reset(rootPath, code, possibleIndexes, parentImportSymbols, hasSideEffects = false, isCommonJsImported = false) { - this.parentImportSymbols = parentImportSymbols || []; + this.parentImportSymbols = parentImportSymbols || {}; this.hasSideEffects = hasSideEffects; this.requireIsBound = false; this.isCommonJsImported = false; @@ -237,7 +239,8 @@ const importedIdentifierVisitor = new (class extends Visitor { const secondArg = args[1]; // if every prop is an string literal, we have something like: export { a, b, c } from './teste'; if (secondArg.properties.every(({value}) => isStringLiteral(value)) && !this.hasSideEffects) { - if((this.parentImportSymbols.deps || []).includes("createUnarySpacing")){ + const parentDeps = this.parentImportSymbols.deps || []; + if(parentDeps.includes("createUnarySpacing")){ // debugger; } // in this case, we need to verify the parent imports to make sure we follow the right tree path @@ -252,7 +255,7 @@ const importedIdentifierVisitor = new (class extends Visitor { if(!this.parentImportSymbols.deps){ debugger; } - return this.parentImportSymbols.deps.includes(value.value) || this.parentImportSymbols.deps.includes("*"); + return parentDeps.includes(value.value) || parentDeps.includes("*"); }); const isSomeSymbolImported = this.parentImportSymbols ? this.parentImportSymbols.sideEffects || isImportedByParent @@ -317,7 +320,7 @@ const importedIdentifierVisitor = new (class extends Visitor { } }); -export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = new Set(), resolveMap) { +export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = new Set(), resolveMap, arch) { const possibleIndexes = findPossibleIndexes(source, [ "export", "exportDefault", @@ -330,18 +333,19 @@ export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = } const ast = tryToParse(source, hash); - removeUnusedExportsVisitor.visit(ast, source, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap); + removeUnusedExportsVisitor.visit(ast, source, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, arch); const newSource = generate(ast, getDefaultOptions()).code; return {source: newSource, madeChanges: removeUnusedExportsVisitor.madeChanges}; } const removeUnusedExportsVisitor = new (class extends Visitor { - reset(rootPath, code, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap) { + reset(rootPath, code, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, arch) { this.madeChanges = false; this.exportInfo = exportInfo; this.allFilesOnBundle = allFilesOnBundle; - this.resolveMap = resolveMap; + this.resolveMap = resolveMap || new Map(); + this.arch = arch; // Defining this.possibleIndexes causes the Visitor to ignore any // subtrees of the AST that do not contain any indexes of identifiers @@ -413,6 +417,10 @@ const removeUnusedExportsVisitor = new (class extends Visitor { const absPath = this.resolveMap.get(firstArg.value); const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; if (!fileIsInBundle) { + // we don't want to remove any native node module import, as they are not bundled in the server bundle + if(Resolver.isNative(firstArg.value) && archMatches(this.arch, "os")){ + return; + } path.replace({ type: "BooleanLiteral", value: false, diff --git a/tools/isobuild/linker.js b/tools/isobuild/linker.js index 67694413405..ed1d46e98af 100644 --- a/tools/isobuild/linker.js +++ b/tools/isobuild/linker.js @@ -1189,6 +1189,7 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, { var headerContent = (new Array(headerLines + 1).join(';')); return _.map(prelinkedFiles, function (file) { + if (file.dynamic) { return file; } @@ -1200,6 +1201,7 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, { var sourceMap = file.sourceMap; sourceMap.mappings = headerContent + sourceMap.mappings; + return { source: header + file.source + footer, sourcePath: file.sourcePath, diff --git a/tools/isobuild/package-source.js b/tools/isobuild/package-source.js index 2df0d19e701..32481d9e4cb 100644 --- a/tools/isobuild/package-source.js +++ b/tools/isobuild/package-source.js @@ -897,7 +897,11 @@ _.extend(PackageSource.prototype, { self.name = null; self.sourceRoot = appDir; self.serveRoot = '/'; - const sideEffects = JSON.parse(files.readFile(projectContext.meteorConfig.packageJsonPath)).sideEffects || false; + let sideEffects = true; + + try { + sideEffects = JSON.parse(files.readFile(projectContext.meteorConfig.packageJsonPath)).sideEffects || false; + }catch(e){} self.sideEffects = sideEffects; // Determine used packages. Note that these are the same for all arches, diff --git a/tools/tests/apps/app-config/.meteor/versions b/tools/tests/apps/app-config/.meteor/versions index 23b1d176167..193a74694e5 100644 --- a/tools/tests/apps/app-config/.meteor/versions +++ b/tools/tests/apps/app-config/.meteor/versions @@ -1,64 +1,55 @@ -allow-deny@1.1.0 -autoupdate@1.4.0 -babel-compiler@7.0.5 -babel-runtime@1.2.2 -base64@1.0.11 -binary-heap@1.0.10 +autoupdate@1.6.0 +babel-compiler@7.5.3 +babel-runtime@1.5.0 +base64@1.0.12 blaze-tools@1.0.10 -boilerplate-generator@1.5.0 -caching-compiler@1.1.11 -caching-html-compiler@1.1.2 -callback-hook@1.1.0 -check@1.3.0 +boilerplate-generator@1.7.0 +caching-compiler@1.2.2 +caching-html-compiler@1.1.3 +callback-hook@1.3.0 +check@1.3.1 ddp@1.4.0 -ddp-client@2.3.1 +ddp-client@2.3.3 ddp-common@1.4.0 -ddp-server@2.1.2 -diff-sequence@1.1.0 -dynamic-import@0.3.0 -ecmascript@0.10.5 -ecmascript-runtime@0.5.0 -ecmascript-runtime-client@0.6.2 -ecmascript-runtime-server@0.5.0 -ejson@1.1.0 -es5-shim@4.7.3 -geojson-utils@1.0.10 +ddp-server@2.3.2 +diff-sequence@1.1.1 +dynamic-import@0.5.2 +ecmascript@0.14.3 +ecmascript-runtime@0.7.0 +ecmascript-runtime-client@0.11.0 +ecmascript-runtime-server@0.10.0 +ejson@1.1.1 +es5-shim@4.8.0 +fetch@0.1.1 hot-code-push@1.0.4 html-tools@1.0.11 htmljs@1.0.11 -http@1.4.0 id-map@1.1.0 -less@2.7.12 +inter-process-messaging@0.1.1 +less@3.0.1 livedata@1.0.18 -logging@1.1.19 -meteor@1.8.4 -meteor-base@1.3.0 -minifier-css@1.3.1 -minifier-js@2.3.3 -minimongo@1.4.3 -modules@0.11.5 -modules-runtime@0.9.2 -mongo@1.4.4 -mongo-dev-server@1.1.0 -mongo-id@1.0.6 -npm-mongo@2.2.33 -ordered-dict@1.1.0 -promise@0.10.2 -random@1.1.0 -reload@1.2.0 +logging@1.1.20 +meteor@1.9.3 +meteor-base@1.4.0 +minifier-css@1.5.2 +minifier-js@2.6.0 +modern-browsers@0.1.5 +modules@0.15.0 +modules-runtime@0.12.0 +mongo-id@1.0.7 +promise@0.11.2 +random@1.2.0 +reload@1.3.0 retry@1.1.0 -routepolicy@1.0.12 -server-render@0.3.0 -shell-server@0.3.1 -shim-common@0.1.0 -socket-stream-client@0.1.0 +routepolicy@1.1.0 +shell-server@0.5.0 +socket-stream-client@0.3.1 spacebars-compiler@1.1.3 -standard-minifier-css@1.4.1 -standard-minifier-js@2.3.2 +standard-minifier-css@1.6.0 +standard-minifier-js@2.6.0 static-html@1.2.2 templating-tools@1.1.2 tracker@1.2.0 underscore@1.0.10 -url@1.2.0 -webapp@1.5.0 +webapp@1.9.1 webapp-hashing@1.0.9 diff --git a/tools/tests/apps/app-config/package-lock.json b/tools/tests/apps/app-config/package-lock.json index 38cde11c9c6..4781bcce7ed 100644 --- a/tools/tests/apps/app-config/package-lock.json +++ b/tools/tests/apps/app-config/package-lock.json @@ -176,9 +176,9 @@ "integrity": "sha1-u5NdSFgsuhaMBoNJV6VKPgcSTxE=" }, "meteor-node-stubs": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/meteor-node-stubs/-/meteor-node-stubs-1.0.0.tgz", - "integrity": "sha512-QJwyv23wyXD3uEMzk5Xr/y5ezoVlCbHvBbrgdkVadn84dmifLRbs0PtD6EeNw5NLIk+SQSfxld7IMdEsneGz5w==", + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/meteor-node-stubs/-/meteor-node-stubs-1.0.1.tgz", + "integrity": "sha512-I4PE/z7eAl45XEsebHA4pcQbgjqEdK3EBGgiUoIZBi3bMQcMq6blLWZo+WdtK4Or9X4NJOiYWw4GmHiubr3egA==", "requires": { "assert": "^1.4.1", "browserify-zlib": "^0.2.0", @@ -414,7 +414,7 @@ "bundled": true }, "elliptic": { - "version": "6.4.1", + "version": "6.5.3", "bundled": true, "requires": { "bn.js": "^4.4.0", From cc6e08b049c9b0ef731ff6fa8931b9be42c5dca9 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 29 Sep 2020 11:20:24 -0300 Subject: [PATCH 15/35] Remove log utils from tree shaking branch --- packages/minifier-js/minifier.js | 6 ------ tools/isobuild/bundler.js | 1 - tools/isobuild/compiler-plugin.js | 4 ---- tools/isobuild/compiler.js | 3 --- tools/isobuild/isopack-cache.js | 3 --- tools/isobuild/linker.js | 9 --------- tools/isobuild/package-api.js | 1 - tools/packaging/catalog/catalog-local.js | 6 ------ 8 files changed, 33 deletions(-) diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index ec67df58513..628b8da6877 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -50,7 +50,6 @@ meteorJsMinify = function (source, options) { try { //TODO: uncomment this code // var optimizedCode = Babel.replaceMeteorInternalState(source, globalDefsMapping) - console.log("Starting minify proccess"); var terserResult = Meteor.wrapAsync(callback => terser.minify(source, { compress: { drop_debugger: false, @@ -75,11 +74,6 @@ meteorJsMinify = function (source, options) { new Error("unknown terser.minify failure"); } } catch (e) { - console.log(e); - Npm.require("fs").writeFile('helloworld.js', source, function (err) { - if (err) return console.log(err); - console.log('Hello World > helloworld.txt'); - }); // Although Babel.minify can handle a wider variety of ECMAScript // 2015+ syntax, it is substantially slower than UglifyJS/terser, so // we use it only as a fallback. diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 12975d1bca9..7657db9eab8 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -3284,7 +3284,6 @@ function bundle({ isopackCache: projectContext.isopackCache, includeCordovaUnibuild: projectContext.platformList.usesCordova() }); - console.log(app); const mergeAppWatchSets = () => { var projectAndLocalPackagesWatchSet = diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index f429e59c305..1aa91dc5030 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -145,9 +145,6 @@ export class CompilerPluginProcessor { var sourceProcessorsWithSlots = {}; var sourceBatches = _.map(self.unibuilds, function (unibuild) { - if(unibuild.pkg.name === 'base64') { - console.log(`runCompilerPlugins -> ${unibuild.pkg.name} ${unibuild.pkg.sideEffects}`); - } const { pkg: { name }, arch } = unibuild; const sourceRoot = name @@ -1445,7 +1442,6 @@ export class PackageSourceBatch { scannerMap.forEach((scanner, name) => { const isApp = ! name; const outputFiles = scanner.getOutputFiles(); - console.log(outputFiles.map(({absPath}) => absPath).includes("/Users/renancamargodecastro/meteor-projects/material-ui-teste/node_modules/react-is/index.js")); const entry = map.get(name); diff --git a/tools/isobuild/compiler.js b/tools/isobuild/compiler.js index 08d4dde3b0c..84998959497 100644 --- a/tools/isobuild/compiler.js +++ b/tools/isobuild/compiler.js @@ -958,9 +958,6 @@ compiler.eachUsedUnibuild = function ( } var usedPackage = isopackCache.getIsopack(use.package); - if(usedPackage.name === 'base64') { - console.log(`eachUsedUnibuild - isopack -> ${usedPackage.sideEffects}`); - } // Ignore this package if we were told to skip debug-only packages and it is // debug-only. diff --git a/tools/isobuild/isopack-cache.js b/tools/isobuild/isopack-cache.js index 68860edcf8d..4a0625ec2e5 100644 --- a/tools/isobuild/isopack-cache.js +++ b/tools/isobuild/isopack-cache.js @@ -209,9 +209,6 @@ export class IsopackCache { }; var packageInfo = self._packageMap.getInfo(name); - if(name === 'base64') { - console.log(`ensurePackageLoaded - isopackCache -> ${packageInfo}`); - } if (! packageInfo) { throw Error("Depend on unknown package " + name + "?"); diff --git a/tools/isobuild/linker.js b/tools/isobuild/linker.js index ed1d46e98af..a219e8bc643 100644 --- a/tools/isobuild/linker.js +++ b/tools/isobuild/linker.js @@ -746,9 +746,6 @@ const getPrelinkedOutputCached = require("optimism").wrap( map: file.sourceMap || null, }; - if(file.servePath.includes("createGenerateClassName/index.js")){ - console.log(file.source); - } var chunks = []; var pathNoSlash = convertColons(file.servePath.replace(/^\//, "")); @@ -1195,9 +1192,6 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, { } if (file.sourceMap) { - if(file.absModuleId && file.absModuleId.includes("createGenerateClassName")){ - console.log("INCLUDE"); - } var sourceMap = file.sourceMap; sourceMap.mappings = headerContent + sourceMap.mappings; @@ -1209,9 +1203,6 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, { sourceMap: sourceMap }; } else { - if(file.absModuleId && file.absModuleId.includes("createGenerateClassName")){ - console.log("INCLUDE"); - } return { source: header + file.source + footer, sourcePath: file.sourcePath, diff --git a/tools/isobuild/package-api.js b/tools/isobuild/package-api.js index fcf1699f18c..b344cfa4809 100644 --- a/tools/isobuild/package-api.js +++ b/tools/isobuild/package-api.js @@ -424,7 +424,6 @@ export class PackageAPI { setSideEffects(sideEffects){ this.sideEffects = sideEffects; - console.log(`Setting side effects ${this.sideEffects}`); } /** diff --git a/tools/packaging/catalog/catalog-local.js b/tools/packaging/catalog/catalog-local.js index 3c7a1c9e0fd..3368108aa10 100644 --- a/tools/packaging/catalog/catalog-local.js +++ b/tools/packaging/catalog/catalog-local.js @@ -352,9 +352,6 @@ _.extend(LocalCatalog.prototype, { initFromPackageDirOptions.name = definiteName; } packageSource.initFromPackageDir(packageDir, initFromPackageDirOptions); - if(packageSource.name === 'base64'){ - console.log(`initSourceFromDir -> ${packageSource.sideEffects}`); - } if (buildmessage.jobHasMessages()) return; // recover by ignoring @@ -418,9 +415,6 @@ _.extend(LocalCatalog.prototype, { var self = this; if (! _.has(self.packages, name)) return null; - if(!name){ - console.log(`getPackageSource -> ${self.packages[name].packageSource.sideEffects}`); - } return self.packages[name].packageSource; } }); From 2bb3cf8d38924e5f1962d38cc6a45bcd03e4e480 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 29 Sep 2020 15:26:10 -0300 Subject: [PATCH 16/35] Remove log utils from tree shaking branch --- tools/isobuild/isopack.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/isobuild/isopack.js b/tools/isobuild/isopack.js index 7c49a56a9d6..b9d8771136a 100644 --- a/tools/isobuild/isopack.js +++ b/tools/isobuild/isopack.js @@ -1023,9 +1023,6 @@ _.extend(Isopack.prototype, { var outputPath = outputDir; var builder = new Builder({ outputPath: outputPath }); - if(self.name === 'base64') { - console.log(`saveToPath - isopack -> ${self.sideEffects}`); - } try { var mainJson = { name: self.name, From 43c38d84d823e568373546a9414ea8797073c0c5 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 29 Sep 2020 16:55:41 -0300 Subject: [PATCH 17/35] Remove log utils from tree shaking branch --- tools/isobuild/js-analyze.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index aa0caeacabe..83bdb643d72 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -363,7 +363,6 @@ const removeUnusedExportsVisitor = new (class extends Visitor { type: "BooleanLiteral", value: false, }) - console.log(`Removing Default export`) this.madeChanges = true; return; } else if (defaultExport) { @@ -377,9 +376,6 @@ const removeUnusedExportsVisitor = new (class extends Visitor { const exportKey = property.key.value || property.key.name; const exportInfoSafe = this.exportInfo?.deps || []; let returnValue = exportInfoSafe.includes(exportKey) || exportInfoSafe.includes("*") ? property : null; - if (!returnValue) { - console.log(`Removing ${exportKey}`) - } return returnValue; }).filter(Boolean) this.madeChanges = true; From b7122624e3d6e80826a8bf2f3c5bfba5a671d45c Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 6 Oct 2020 11:35:05 -0300 Subject: [PATCH 18/35] Implement better logic for getting side effects. If APP doesn't say it is side effect free, don't even try. --- tools/isobuild/bundler.js | 22 ++++++++++++++----- tools/isobuild/linker.js | 8 +++---- .../imports/links/jsx-import-test/parent.js | 2 +- tools/tests/apps/modules/package-lock.json | 8 +++---- tools/tests/apps/modules/package.json | 1 + tools/tests/apps/modules/tests.js | 1 + 6 files changed, 28 insertions(+), 14 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 7657db9eab8..3f7cad691c2 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1136,7 +1136,7 @@ class Target { const isWeb = archinfo.matches(this.arch, 'web'); const isOs = archinfo.matches(this.arch, 'os'); - const { jsOutputFilesMap, resolveMap } = compilerPluginModule.PackageSourceBatch + const { jsOutputFilesMap, resolveMap = new Map() } = compilerPluginModule.PackageSourceBatch .computeJsOutputFilesMap(sourceBatches); sourceBatches.forEach(batch => { @@ -1172,9 +1172,18 @@ class Target { const unibuild = sourceBatch.unibuild; const name = unibuild.pkg.name || null; jsOutputFilesMap.get(name).files.forEach((file) => { - if (!file.imports) return; - Object.entries(file.imports).forEach(([key, object]) => { + const fileResolveMap = resolveMap.get(file.absPath) || new Map(); + Object.entries(file.deps || {}).forEach(([key, depInfo]) => { + if(key.includes("jquery")){ + debugger; + } + const absKey = fileResolveMap.get(key); + const wasImported = importMap.get(absKey) || false; + importMap.set(absKey, {commonJsImported: wasImported || depInfo.commonJsImported}); + }) + Object.entries(file.imports || []).forEach(([key, object]) => { importMap.set(key, { + ...importMap.get(key) || {}, deps: [...new Set([...(importMap.get(key)?.deps || []), ...object.deps])], sideEffects: object.sideEffects }) @@ -1185,6 +1194,9 @@ class Target { const allFilesOnBundle = new Set(Array.from(jsOutputFilesMap).flatMap(([, value]) => { return value.files.map(({absPath}) => absPath); })); + + const appSideEffects = sourceBatches.find((sourceBatch) => !sourceBatch.unibuild?.pkg?.name)?.pkg?.sideEffects ?? true + // now we need to remove the exports, and let the minifier do it's job later sourceBatches.forEach((sourceBatch) => { const unibuild = sourceBatch.unibuild; @@ -1193,7 +1205,7 @@ class Target { // we will assume that every meteor package that exports something is a // side effect true package, this is set on the package-api file when api.export is called // console.log(`${unibuild.pkg.name} sideEffects: ${unibuild.pkg.sideEffects}`) - if(unibuild.pkg.sideEffects && name !== 'modules' || unibuild.arch.includes("legacy")){ + if(appSideEffects || unibuild.pkg.sideEffects && name !== 'modules' || unibuild.arch.includes("legacy")){ return; } jsOutputFilesMap.get(name).files.forEach((file) => { @@ -1201,7 +1213,7 @@ class Target { // if it was commonjsImported we have nothing to do here - if(file.deps?.commonJsImported){ + if(importedSymbolsFromFile?.commonJsImported){ return; } const { source:newSource, madeChanges } = removeUnusedExports( diff --git a/tools/isobuild/linker.js b/tools/isobuild/linker.js index a219e8bc643..f061daeeba8 100644 --- a/tools/isobuild/linker.js +++ b/tools/isobuild/linker.js @@ -267,10 +267,10 @@ _.extend(Module.prototype, { const tree = getTree(file); - // if (file.aliasId) { - // addToTree(file.aliasId, file.absModuleId, tree); - // return; - // } + if (file.aliasId) { + addToTree(file.aliasId, file.absModuleId, tree); + return; + } if (file.isDynamic()) { const servePath = files.pathJoin("dynamic", file.absModuleId); diff --git a/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js b/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js index 3478298bd0e..ba2d46a1eb0 100644 --- a/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js +++ b/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js @@ -1 +1 @@ -export jsx from "./child"; +export { default as jsx } from "./child"; diff --git a/tools/tests/apps/modules/package-lock.json b/tools/tests/apps/modules/package-lock.json index 0bfa075a4a0..e7a310e28f7 100644 --- a/tools/tests/apps/modules/package-lock.json +++ b/tools/tests/apps/modules/package-lock.json @@ -1195,9 +1195,9 @@ } }, "meteor-node-stubs": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/meteor-node-stubs/-/meteor-node-stubs-1.0.0.tgz", - "integrity": "sha512-QJwyv23wyXD3uEMzk5Xr/y5ezoVlCbHvBbrgdkVadn84dmifLRbs0PtD6EeNw5NLIk+SQSfxld7IMdEsneGz5w==", + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/meteor-node-stubs/-/meteor-node-stubs-1.0.1.tgz", + "integrity": "sha512-I4PE/z7eAl45XEsebHA4pcQbgjqEdK3EBGgiUoIZBi3bMQcMq6blLWZo+WdtK4Or9X4NJOiYWw4GmHiubr3egA==", "requires": { "assert": "^1.4.1", "browserify-zlib": "^0.2.0", @@ -1433,7 +1433,7 @@ "bundled": true }, "elliptic": { - "version": "6.4.1", + "version": "6.5.3", "bundled": true, "requires": { "bn.js": "^4.4.0", diff --git a/tools/tests/apps/modules/package.json b/tools/tests/apps/modules/package.json index 0852a5fca0b..225886d2cdc 100644 --- a/tools/tests/apps/modules/package.json +++ b/tools/tests/apps/modules/package.json @@ -3,6 +3,7 @@ "author": "Ben Newman ", "description": "Test app exercising many aspects of the Meteor module system.", "private": true, + "sideEffects": true, "dependencies": { "@babel/core": "^7.8.4", "@babel/plugin-proposal-do-expressions": "^7.8.3", diff --git a/tools/tests/apps/modules/tests.js b/tools/tests/apps/modules/tests.js index 2c91a9e8cc6..635385a7811 100644 --- a/tools/tests/apps/modules/tests.js +++ b/tools/tests/apps/modules/tests.js @@ -588,6 +588,7 @@ describe("symlinking node_modules", () => { it("should not break custom import extensions", () => { import { jsx } from "jsx-import-test"; + console.log(jsx); assert.strictEqual(jsx.type, "div"); assert.strictEqual(jsx.props.children, "oyez"); return import("./imports/links/jsx-import-test/child").then(ns => { From 7d45a7deaf3cf27c8aab5b2047c2a03fada1087a Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 6 Oct 2020 11:45:17 -0300 Subject: [PATCH 19/35] Don't tree shake import if is native module. --- tools/isobuild/js-analyze.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index 83bdb643d72..77893d7db1f 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -414,7 +414,7 @@ const removeUnusedExportsVisitor = new (class extends Visitor { const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; if (!fileIsInBundle) { // we don't want to remove any native node module import, as they are not bundled in the server bundle - if(Resolver.isNative(firstArg.value) && archMatches(this.arch, "os")){ + if(Resolver.isNative(firstArg.value)){ return; } path.replace({ From 58d6b49c874a42dd9277674b41079f7441b7c426 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Wed, 7 Oct 2020 15:45:29 -0300 Subject: [PATCH 20/35] Fix issue with dynamic imports not being included in the deps array. --- tools/isobuild/js-analyze.js | 4 +++- tools/isobuild/resolver.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index 77893d7db1f..d1172a0aeab 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -215,7 +215,9 @@ const importedIdentifierVisitor = new (class extends Visitor { this.addIdentifier(firstArg.value, "require", false, true, true); this.addDependency(firstArg.value, ['*'], true); } else if (node.callee.type === "Import" || - isIdWithName(node.callee, "import")) { + isIdWithName(node.callee, "import") || + node.callee?.property?.name === "dynamicImport" + ) { this.addIdentifier(firstArg.value, "import", true, false, true); this.addDependency(firstArg.value, ['*'], true); } else { diff --git a/tools/isobuild/resolver.ts b/tools/isobuild/resolver.ts index 45d49f7c19e..0092f20fb41 100644 --- a/tools/isobuild/resolver.ts +++ b/tools/isobuild/resolver.ts @@ -105,7 +105,7 @@ export default class Resolver { // import/export in their "module" dependency trees. this.mainFields = ["browser", "main", "module"]; } else { - this.mainFields = ["module", "browser", "main"]; + this.mainFields = ["browser", "module", "main"]; } } else { this.mainFields = ["main"]; From 4291cdbc40b73ea662c3430568f6a3c83d813f4a Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 8 Oct 2020 09:52:04 -0300 Subject: [PATCH 21/35] Import scanner fix for dynamic imports --- tools/isobuild/import-scanner.ts | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index f17a3a2f8d7..b734eca30f8 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -427,21 +427,31 @@ export default class ImportScanner { private filesInfo: Map; - private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo): AnalyzeInfo { + private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo, forDynamicImport: boolean): AnalyzeInfo { /* dependencies?: Record; proxyDependencies?: Record; identifiers?: Record; exports?: [string] */ + if(file?.absModuleId?.includes("console-browserify")){ + debugger; + } file.deps = file.deps || {}; Object.entries(importInfo.identifiers || {}).forEach(([key, value]) => { + const dynamic = this.isWebBrowser() && + (forDynamicImport || + value.parentWasDynamic || + value.dynamic); + if(!file.deps[key]) { - file.deps[key] = value; + file.deps[key] = {...value, dynamic}; return; } + file.deps[key] = { ...file.deps[key], + dynamic, commonJsImported: value.commonJsImported || file.deps[key].commonJsImported, sideEffects: value.sideEffects || file.deps[key].sideEffects }; @@ -838,6 +848,9 @@ export default class ImportScanner { if(!root.status) root.status = ImportTreeNodeStatus.WHITE; const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} if(root.absImportedPath) { + if(root.absImportedPath?.includes("console-browserify")){ + debugger; + } this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo)); } root.status = ImportTreeNodeStatus.GRAY; @@ -852,7 +865,7 @@ export default class ImportScanner { scanImports() { this.outputFiles.forEach(file => { if (! file.lazy) { - const importTree = this.scanFile(file, false, {deps:["*"], sideEffects: this.sideEffects}); + const importTree = this.scanFile(file, file.imported === "dynamic", {deps:["*"], sideEffects: this.sideEffects}); this.addImportTree(importTree); } }); @@ -1228,10 +1241,10 @@ export default class ImportScanner { // this is actually an temporary dependency tree, we will need to analyze the // children to see what they are exporting const cachedFileInfo = this.filesInfo.get(file.absPath) || {}; - mergedInfo = this.mergeImportInfo(cachedFileInfo, importInfo); + mergedInfo = this.mergeImportInfo(cachedFileInfo, importInfo, forDynamicImport); // we only need the local view here, later we will need the global, merged one this.filesInfo.set(file.absPath, mergedInfo); - this.mergeImportInfo(file, importInfo) + this.mergeImportInfo(file, importInfo, forDynamicImport) } catch (e) { if (e.$ParseError) { @@ -1251,6 +1264,9 @@ export default class ImportScanner { depFile: file, }; + if(file?.absModuleId?.includes("console-browserify")){ + debugger; + } // @ts-ignore each(file.deps, (info: ImportInfo, id: string) => { @@ -1286,7 +1302,7 @@ export default class ImportScanner { let depFile = this.getFile(absImportedPath); - const visitFrom = `${file.absPath}->${absImportedPath}` + const visitFrom = `${file.absPath}->${absImportedPath}`; const depHasSideEffects = info.sideEffects || hasSideEffects; if (depFile) { From 61bcc48a626ac0e8b356c84f36fefdc6ba26cf86 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Fri, 9 Oct 2020 17:13:35 -0300 Subject: [PATCH 22/35] Use import status when deciding which tree to follow. Also remove static references when only dynamic remains. --- packages/modules/package.js | 1 - tools/isobuild/bundler.js | 5 +- tools/isobuild/compiler-plugin.js | 16 +++- tools/isobuild/import-scanner.ts | 76 +++++++++++++------ tools/isobuild/js-analyze.js | 23 ++++-- .../apps/dynamic-import/package-lock.json | 2 +- 6 files changed, 84 insertions(+), 39 deletions(-) diff --git a/packages/modules/package.js b/packages/modules/package.js index 9f6404107c3..b697b26ec12 100644 --- a/packages/modules/package.js +++ b/packages/modules/package.js @@ -15,7 +15,6 @@ Package.onUse(function(api) { api.mainModule("client.js", "client"); api.mainModule("server.js", "server"); api.export("meteorInstall"); - api.setSideEffects(false); // When compiling legacy code, the babel-compiler and meteor-babel // packages assume meteorBabelHelpers.sanitizeForInObject is defined. diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 3f7cad691c2..eb7ee20dce5 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1136,7 +1136,7 @@ class Target { const isWeb = archinfo.matches(this.arch, 'web'); const isOs = archinfo.matches(this.arch, 'os'); - const { jsOutputFilesMap, resolveMap = new Map() } = compilerPluginModule.PackageSourceBatch + const { jsOutputFilesMap, resolveMap = new Map(), fileImportState = new Map() } = compilerPluginModule.PackageSourceBatch .computeJsOutputFilesMap(sourceBatches); sourceBatches.forEach(batch => { @@ -1195,7 +1195,7 @@ class Target { return value.files.map(({absPath}) => absPath); })); - const appSideEffects = sourceBatches.find((sourceBatch) => !sourceBatch.unibuild?.pkg?.name)?.pkg?.sideEffects ?? true + const appSideEffects = sourceBatches.find((sourceBatch) => sourceBatch.unibuild.pkg.name == null).unibuild.pkg.sideEffects ?? true // now we need to remove the exports, and let the minifier do it's job later sourceBatches.forEach((sourceBatch) => { @@ -1222,6 +1222,7 @@ class Target { importedSymbolsFromFile, allFilesOnBundle, resolveMap.get(file.absPath) || new Map(), + fileImportState, unibuild.arch ); diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index 1aa91dc5030..da81c04fb24 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -1254,6 +1254,7 @@ export class PackageSourceBatch { static computeJsOutputFilesMap(sourceBatches) { const map = new Map; const resolveMap = new Map(); + const fileImportState = new Map(); sourceBatches.forEach(batch => { const name = batch.unibuild.pkg.name || null; @@ -1326,6 +1327,7 @@ export class PackageSourceBatch { // relocated to a source batch that could handle them. const allRelocatedModules = Object.create(null); const scannerMap = new Map; + const appSideEffects = sourceBatches.find((sourceBatch) => sourceBatch.unibuild.pkg.name == null).unibuild.pkg.sideEffects ?? true sourceBatches.forEach(batch => { const name = batch.unibuild.pkg.name || null; @@ -1348,11 +1350,13 @@ export class PackageSourceBatch { }); const entry = map.get(name); + + const pkgSideEffects = name === 'modules' ? appSideEffects : batch.unibuild.pkg.sideEffects; const scanner = new ImportScanner({ name, bundleArch: batch.processor.arch, - sideEffects: batch.unibuild.pkg.sideEffects, + sideEffects: pkgSideEffects, extensions: batch.importExtensions, sourceRoot: batch.sourceRoot, nodeModulesPaths, @@ -1496,6 +1500,10 @@ export class PackageSourceBatch { entry.files = outputFiles; } + + Object.entries(scanner.importedStatusFile).forEach(function([key, value]) { + fileImportState.set(key, value); + }); scanner.resolveMap.forEach(function(value, key) { const current = resolveMap.get(key) || new Map(); for(const [newKey, newValue] of value.entries()){ @@ -1504,10 +1512,10 @@ export class PackageSourceBatch { resolveMap.set(key, current); }); }); - return this._watchOutputFiles(map, resolveMap); + return this._watchOutputFiles(map, resolveMap, fileImportState); } - static _watchOutputFiles(jsOutputFilesMap, resolveMap) { + static _watchOutputFiles(jsOutputFilesMap, resolveMap, fileImportState) { // Watch all output files produced by computeJsOutputFilesMap. jsOutputFilesMap.forEach(entry => { entry.files.forEach(file => { @@ -1534,7 +1542,7 @@ export class PackageSourceBatch { } }); }); - return { jsOutputFilesMap, resolveMap }; + return { jsOutputFilesMap, resolveMap, fileImportState }; } static _warnAboutMissingModules(missingModules) { diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index b734eca30f8..3ac17163006 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -280,6 +280,7 @@ function setImportedStatus(file: File, status: string | boolean) { if (isHigherStatus(status, file.imported)) { file.imported = status; } + return file.imported; } // Map from SHA (which is already calculated, so free for us) @@ -409,6 +410,7 @@ type ImportTreeNode = { export default class ImportScanner { public name: string; + public importedStatusFile: Record private bundleArch: string; private sourceRoot: string; private sideEffects: boolean; @@ -444,16 +446,18 @@ export default class ImportScanner { value.parentWasDynamic || value.dynamic); - if(!file.deps[key]) { + const current = file.deps[key]; + if(!current) { file.deps[key] = {...value, dynamic}; return; } file.deps[key] = { - ...file.deps[key], - dynamic, - commonJsImported: value.commonJsImported || file.deps[key].commonJsImported, - sideEffects: value.sideEffects || file.deps[key].sideEffects + ...current, + imported: isHigherStatus(file.imported, current.imported) ? file.imported : current.imported, + dynamic: dynamic, + commonJsImported: value.commonJsImported || current.commonJsImported, + sideEffects: value.sideEffects || current.sideEffects }; }); @@ -498,6 +502,7 @@ export default class ImportScanner { this.name = name; this.resolveMap = new Map(); this.filesInfo = new Map(); + this.importedStatusFile = {}; this.visitedFromRootsCache = {}; this.bundleArch = bundleArch; this.sideEffects = sideEffects; @@ -851,7 +856,7 @@ export default class ImportScanner { if(root.absImportedPath?.includes("console-browserify")){ debugger; } - this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo)); + this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo, {imported: this.importedStatusFile[root.absImportedPath]})); } root.status = ImportTreeNodeStatus.GRAY; @@ -1179,10 +1184,12 @@ export default class ImportScanner { // they can be handled by the loop above. const file = this.getFile(resolved.path); if (file && file.alias) { - setImportedStatus(file, forDynamicImport ? Status.DYNAMIC : Status.STATIC); + this.setImportedStatusGlobal(file, forDynamicImport ? Status.DYNAMIC : Status.STATIC); return file.alias; } } + + return resolved; } @@ -1206,7 +1213,8 @@ export default class ImportScanner { forDynamicImport: boolean = false, parentImportSymbols?: ImportIdentifiersEntry, isCommonJsImported: boolean = false, - treeHasSideEffects: boolean = false) : ImportTreeNode { + treeHasSideEffects: boolean = false, + ) : ImportTreeNode { const hasSideEffects = treeHasSideEffects || this.sideEffects || file.sideEffects; @@ -1214,7 +1222,7 @@ export default class ImportScanner { // Set file.imported to a truthy value (either "dynamic" or true). //TODO: we need a better logic to stop scanning a file. // if we first find a usage that gets it partially first, then we will include less than needed at the end - setImportedStatus(file, forDynamicImport ? Status.DYNAMIC : Status.STATIC); + this.setImportedStatusGlobal(file, forDynamicImport ? Status.DYNAMIC : Status.STATIC); if (file.reportPendingErrors && file.reportPendingErrors() > 0) { @@ -1263,10 +1271,8 @@ export default class ImportScanner { absImportedPath: file.absPath, depFile: file, }; + const fromFilePath = `${file.absPath}(${file.imported})`; - if(file?.absModuleId?.includes("console-browserify")){ - debugger; - } // @ts-ignore each(file.deps, (info: ImportInfo, id: string) => { @@ -1276,8 +1282,8 @@ export default class ImportScanner { // it's better if forDynamicImport never becomes true on the server. const dynamic = this.isWebBrowser() && (forDynamicImport || - info.parentWasDynamic || - info.dynamic); + file.imported === Status.DYNAMIC || + info.dynamic) || false; const resolved = this.resolve(file, id, dynamic); const absImportedPath = resolved && resolved !== "missing" && resolved.path; @@ -1302,7 +1308,7 @@ export default class ImportScanner { let depFile = this.getFile(absImportedPath); - const visitFrom = `${file.absPath}->${absImportedPath}`; + const visitFrom = `${fromFilePath}->${absImportedPath}`; const depHasSideEffects = info.sideEffects || hasSideEffects; if (depFile) { @@ -1324,11 +1330,11 @@ export default class ImportScanner { } } - const parentImports = file.imports && file.imports[absImportedPath]; + const parentImports: ImportIdentifiersEntry = file.imports && file.imports[absImportedPath] || {}; - if(!(this.visitedFromRoots[absImportedPath] || []).includes(file.absPath)) { - if(!this.visitedFromRoots[absImportedPath]) this.visitedFromRoots[absImportedPath] = []; - this.visitedFromRoots[absImportedPath].push(file.absPath); + if(!this.visitedFromRoots[absImportedPath]?.includes(fromFilePath)) { + if(!this.visitedFromRoots[absImportedPath]){ this.visitedFromRoots[absImportedPath] = [] }; + this.visitedFromRoots[absImportedPath].push(fromFilePath); const importTreeNode = this.scanFile( depFile, dynamic, @@ -1348,11 +1354,14 @@ export default class ImportScanner { if (! depFile) { return; } + if(depFile.alias) { + console.log(`alias: ${JSON.stringify(depFile.alias)}`); + } const parentImports = file.imports && file.imports[absImportedPath]; - if(!(this.visitedFromRoots[absImportedPath] || []).includes(file.absPath)) { + if(!(this.visitedFromRoots[absImportedPath] || []).includes(fromFilePath)) { if(!this.visitedFromRoots[absImportedPath]) this.visitedFromRoots[absImportedPath] = []; - this.visitedFromRoots[absImportedPath].push(file.absPath); + this.visitedFromRoots[absImportedPath].push(fromFilePath); childrenDep = this.scanFile( depFile, dynamic, @@ -1372,13 +1381,19 @@ export default class ImportScanner { depFile, }; + // if(absImportedPath.includes("@apollo/react-ssr")){ + // debugger + // } + + const hasAnyImports = !!(file.imports || {})[absImportedPath]; if(file.proxyImports && file.proxyImports[id] && parentImportSymbols && !isCommonJsImported && !info.commonJsImported && !this.sideEffects && - !depHasSideEffects){ + !depHasSideEffects && + !hasAnyImports){ // if we are doing an wildcard export, it's time to make sure the file we included in the bundle // should really be here @@ -1397,7 +1412,7 @@ export default class ImportScanner { } }); - this.visitedFromRootsCache[file.absPath] = rootChildren; + this.visitedFromRootsCache[fromFilePath] = rootChildren; return rootChildren; } @@ -1840,6 +1855,13 @@ export default class ImportScanner { return null; } + private setImportedStatusGlobal(file: File, status: string | boolean){ + const newStatus = setImportedStatus(file, status); + if(isHigherStatus(newStatus, this.importedStatusFile[file.absPath])){ + this.importedStatusFile[file.absPath] = newStatus; + } + } + private addPkgJsonToOutput( pkgJsonPath: string, pkg: Record, @@ -1850,7 +1872,7 @@ export default class ImportScanner { if (file) { // If the file already exists, just update file.imported according // to the forDynamicImport parameter. - setImportedStatus(file, forDynamicImport ? Status.DYNAMIC : Status.STATIC); + this.setImportedStatusGlobal(file, forDynamicImport ? Status.DYNAMIC : Status.STATIC); return file; } @@ -1901,6 +1923,7 @@ export default class ImportScanner { const deps = pkgFile.deps || (pkgFile.deps = Object.create(null)); const absPkgJsonPath = pathJoin(this.sourceRoot, pkgFile.sourcePath); + console.log(`resolvePkgJsonBrowserAliases: ${JSON.stringify(browser)}`); Object.keys(browser).forEach(sourceId => { deps[sourceId] = deps[sourceId] || {}; @@ -1918,6 +1941,10 @@ export default class ImportScanner { } const sourceAbsModuleId = this.getAbsModuleId(source.path); + console.log(`sourceAbsModuleId: ${sourceAbsModuleId}`); + if(sourceAbsModuleId === '/node_modules/meteor-node-stubs/node_modules/assert/node_modules/util/support/isBuffer.js'){ + debugger; + } if (! sourceAbsModuleId || ! pkgFile.absModuleId) { return; } @@ -1966,6 +1993,7 @@ export default class ImportScanner { } if (file) { + console.log(`found file: ${alias}`); file.alias = alias; } else { const relSourcePath = pathRelative(this.sourceRoot, source.path); diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index d1172a0aeab..c3cadadc48b 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -1,11 +1,10 @@ -import {parse, getDefaultOptions} from 'meteor-babel'; +import {getDefaultOptions, parse} from 'meteor-babel'; import generate from '@babel/generator'; import {analyze as analyzeScope} from 'escope'; import LRU from "lru-cache"; import Visitor from "reify/lib/visitor.js"; import {findPossibleIndexes} from "reify/lib/utils.js"; -import {matches as archMatches} from "../utils/archinfo"; import Resolver from "./resolver"; const hasOwn = Object.prototype.hasOwnProperty; @@ -322,7 +321,7 @@ const importedIdentifierVisitor = new (class extends Visitor { } }); -export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = new Set(), resolveMap, arch) { +export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = new Set(), resolveMap, fileImportState, arch) { const possibleIndexes = findPossibleIndexes(source, [ "export", "exportDefault", @@ -335,18 +334,19 @@ export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = } const ast = tryToParse(source, hash); - removeUnusedExportsVisitor.visit(ast, source, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, arch); + removeUnusedExportsVisitor.visit(ast, source, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, fileImportState, arch); const newSource = generate(ast, getDefaultOptions()).code; return {source: newSource, madeChanges: removeUnusedExportsVisitor.madeChanges}; } const removeUnusedExportsVisitor = new (class extends Visitor { - reset(rootPath, code, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, arch) { + reset(rootPath, code, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, fileImportState, arch) { this.madeChanges = false; this.exportInfo = exportInfo; this.allFilesOnBundle = allFilesOnBundle; this.resolveMap = resolveMap || new Map(); + this.fileImportState = fileImportState || new Map(); this.arch = arch; // Defining this.possibleIndexes causes the Visitor to ignore any @@ -377,8 +377,7 @@ const removeUnusedExportsVisitor = new (class extends Visitor { const exportKey = property.key.value || property.key.name; const exportInfoSafe = this.exportInfo?.deps || []; - let returnValue = exportInfoSafe.includes(exportKey) || exportInfoSafe.includes("*") ? property : null; - return returnValue; + return exportInfoSafe.includes(exportKey) || exportInfoSafe.includes("*") ? property : null; }).filter(Boolean) this.madeChanges = true; } @@ -414,6 +413,16 @@ const removeUnusedExportsVisitor = new (class extends Visitor { if(node.arguments.length <= 1) return; const absPath = this.resolveMap.get(firstArg.value); const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; + if(fileIsInBundle && isPropertyWithName(node.callee.property, "link")){ + // we are seeing a static import, but this import might have been "tree-shaked", remaining only the dynamic import + const importStatus = this.fileImportState.get(absPath); + if(importStatus === 'dynamic' || !importStatus){ + path.replace({ + type: "BooleanLiteral", + value: false, + }); + } + } if (!fileIsInBundle) { // we don't want to remove any native node module import, as they are not bundled in the server bundle if(Resolver.isNative(firstArg.value)){ diff --git a/tools/tests/apps/dynamic-import/package-lock.json b/tools/tests/apps/dynamic-import/package-lock.json index 53af0c9685a..dc4ac173b7f 100644 --- a/tools/tests/apps/dynamic-import/package-lock.json +++ b/tools/tests/apps/dynamic-import/package-lock.json @@ -434,7 +434,7 @@ "bundled": true }, "elliptic": { - "version": "6.4.1", + "version": "6.5.3", "bundled": true, "requires": { "bn.js": "^4.4.0", From e8c2880fa08a6c7777262e450abc40cabe0dbeb8 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Wed, 14 Oct 2020 19:04:31 -0300 Subject: [PATCH 23/35] Fix issue with dynamic imports not being included in the deps array. --- tools/isobuild/bundler.js | 20 +++++++----- tools/isobuild/compiler-plugin.js | 6 +++- tools/isobuild/import-scanner.ts | 47 +++++++++++++++------------- tools/isobuild/js-analyze.js | 52 ++++++++++++++++++++----------- tools/isobuild/linker.js | 4 +++ 5 files changed, 80 insertions(+), 49 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index eb7ee20dce5..bb7982c9a2c 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1174,9 +1174,6 @@ class Target { jsOutputFilesMap.get(name).files.forEach((file) => { const fileResolveMap = resolveMap.get(file.absPath) || new Map(); Object.entries(file.deps || {}).forEach(([key, depInfo]) => { - if(key.includes("jquery")){ - debugger; - } const absKey = fileResolveMap.get(key); const wasImported = importMap.get(absKey) || false; importMap.set(absKey, {commonJsImported: wasImported || depInfo.commonJsImported}); @@ -1195,7 +1192,13 @@ class Target { return value.files.map(({absPath}) => absPath); })); - const appSideEffects = sourceBatches.find((sourceBatch) => sourceBatch.unibuild.pkg.name == null).unibuild.pkg.sideEffects ?? true + const sourceBatch = sourceBatches.find((sourceBatch) => { + const name = sourceBatch.unibuild.pkg.name || null; + return !name; + }); + const appSideEffects = sourceBatch ? sourceBatch.unibuild.pkg.sideEffects : true; + + console.log(`appSideEffects: ${appSideEffects}`); // now we need to remove the exports, and let the minifier do it's job later sourceBatches.forEach((sourceBatch) => { @@ -1216,9 +1219,11 @@ class Target { if(importedSymbolsFromFile?.commonJsImported){ return; } - const { source:newSource, madeChanges } = removeUnusedExports( - file.dataString, - file.hash, + if(file.absPath && file.absPath.includes("node_modules/acorn/dist/acorn.mjs")){ + debugger; + } + const { source: newSource, map, madeChanges } = removeUnusedExports( + file, importedSymbolsFromFile, allFilesOnBundle, resolveMap.get(file.absPath) || new Map(), @@ -1231,6 +1236,7 @@ class Target { file.data = Buffer.from(newSource, "utf8"); file.source = Buffer.from(newSource, "utf8"); file.hash = sha1(file.data); + file.sourceMap = map; } }) }) diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index da81c04fb24..986e1906ae2 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -1327,7 +1327,11 @@ export class PackageSourceBatch { // relocated to a source batch that could handle them. const allRelocatedModules = Object.create(null); const scannerMap = new Map; - const appSideEffects = sourceBatches.find((sourceBatch) => sourceBatch.unibuild.pkg.name == null).unibuild.pkg.sideEffects ?? true + const sourceBatch = sourceBatches.find((sourceBatch) => { + const name = sourceBatch.unibuild.pkg.name || null; + return !name; + }); + const appSideEffects = sourceBatch ? sourceBatch.unibuild.pkg.sideEffects : true; sourceBatches.forEach(batch => { const name = batch.unibuild.pkg.name || null; diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 3ac17163006..6cb0647b007 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -336,8 +336,8 @@ interface RawFile { } interface ImportIdentifiersEntry { - deps: [string]; - sideEffects: boolean; + deps?: [string]; + sideEffects?: boolean; } interface AnalyzeInfo { @@ -436,26 +436,18 @@ export default class ImportScanner { identifiers?: Record; exports?: [string] */ - if(file?.absModuleId?.includes("console-browserify")){ - debugger; - } file.deps = file.deps || {}; Object.entries(importInfo.identifiers || {}).forEach(([key, value]) => { - const dynamic = this.isWebBrowser() && - (forDynamicImport || - value.parentWasDynamic || - value.dynamic); const current = file.deps[key]; if(!current) { - file.deps[key] = {...value, dynamic}; + file.deps[key] = {...value}; return; } file.deps[key] = { ...current, imported: isHigherStatus(file.imported, current.imported) ? file.imported : current.imported, - dynamic: dynamic, commonJsImported: value.commonJsImported || current.commonJsImported, sideEffects: value.sideEffects || current.sideEffects }; @@ -853,7 +845,7 @@ export default class ImportScanner { if(!root.status) root.status = ImportTreeNodeStatus.WHITE; const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} if(root.absImportedPath) { - if(root.absImportedPath?.includes("console-browserify")){ + if(root?.absImportedPath?.includes("uuid")){ debugger; } this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo, {imported: this.importedStatusFile[root.absImportedPath]})); @@ -932,7 +924,7 @@ export default class ImportScanner { // the _scanFile method, which is important because this file // doesn't have a .data or .dataString property. deps: { [id]: staticImportInfo } - }, false); // !forDynamicImport + }, false, {}, false, this.sideEffects); // !forDynamicImport this.addImportTree(tree); } @@ -941,7 +933,7 @@ export default class ImportScanner { ...fakeStub, exports: ["*"], sideEffects: this.sideEffects, deps: { [id]: dynamicImportInfo }, - }, true); // forDynamicImport + }, true, {}, true, this.sideEffects); // forDynamicImport this.addImportTree(tree); } }); @@ -1217,7 +1209,7 @@ export default class ImportScanner { ) : ImportTreeNode { - const hasSideEffects = treeHasSideEffects || this.sideEffects || file.sideEffects; + const hasSideEffects = treeHasSideEffects || this.sideEffects; // Set file.imported to a truthy value (either "dynamic" or true). //TODO: we need a better logic to stop scanning a file. @@ -1239,6 +1231,10 @@ export default class ImportScanner { let importInfo; let mergedInfo : AnalyzeInfo; + if(file.absPath?.includes("@material-ui/icons/esm/index.js")){ + debugger + } + try { importInfo = this.findImportedModuleIdentifiers( file, @@ -1265,6 +1261,10 @@ export default class ImportScanner { } throw e; } + if(file.absModuleId?.includes("console")){ + debugger; + } + const rootChildren : ImportTreeNode = { children: [], @@ -1281,9 +1281,9 @@ export default class ImportScanner { // browser (even though it works equally well on the server), so // it's better if forDynamicImport never becomes true on the server. const dynamic = this.isWebBrowser() && - (forDynamicImport || - file.imported === Status.DYNAMIC || - info.dynamic) || false; + (forDynamicImport || + info.parentWasDynamic || + info.dynamic); const resolved = this.resolve(file, id, dynamic); const absImportedPath = resolved && resolved !== "missing" && resolved.path; @@ -1354,10 +1354,13 @@ export default class ImportScanner { if (! depFile) { return; } + + if(depFile.alias) { console.log(`alias: ${JSON.stringify(depFile.alias)}`); } + const parentImports = file.imports && file.imports[absImportedPath]; if(!(this.visitedFromRoots[absImportedPath] || []).includes(fromFilePath)) { if(!this.visitedFromRoots[absImportedPath]) this.visitedFromRoots[absImportedPath] = []; @@ -1381,9 +1384,9 @@ export default class ImportScanner { depFile, }; - // if(absImportedPath.includes("@apollo/react-ssr")){ - // debugger - // } + if(absImportedPath.includes("@material-ui/icons/esm/index.js")){ + debugger + } const hasAnyImports = !!(file.imports || {})[absImportedPath]; if(file.proxyImports && @@ -1402,7 +1405,7 @@ export default class ImportScanner { const isSomeSymbolImported = parentImportSymbols.deps.some((symbol) => { // @ts-ignore return depFileImportInfo?.includes(file.proxyImports[id][symbol]) || depFileImportInfo?.includes(symbol); - }) || parentImportSymbols.deps.includes("*"); + }) || parentImportSymbols.deps?.includes("*"); if(isSomeSymbolImported){ rootChildren.children?.push(depRoot); } diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index c3cadadc48b..c87f9669410 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -215,15 +215,14 @@ const importedIdentifierVisitor = new (class extends Visitor { this.addDependency(firstArg.value, ['*'], true); } else if (node.callee.type === "Import" || isIdWithName(node.callee, "import") || - node.callee?.property?.name === "dynamicImport" + isPropertyWithName(node.callee.property, "dynamicImport") ) { - this.addIdentifier(firstArg.value, "import", true, false, true); + this.addIdentifier(firstArg.value, "import", true, true, true); this.addDependency(firstArg.value, ['*'], true); } else { if (isModuleUsage) { const isImport = - isPropertyWithName(node.callee.property, "link") || - isPropertyWithName(node.callee.property, "dynamicImport"); + isPropertyWithName(node.callee.property, "link"); if (isImport) { // if we have an object definition on module.link(), we are importing with ES6 possible without side effects @@ -233,7 +232,9 @@ const importedIdentifierVisitor = new (class extends Visitor { this.addIdentifier( firstArg.value, "import", - isImport === "dynamicImport" + false, + false, + true ); return; } @@ -267,7 +268,9 @@ const importedIdentifierVisitor = new (class extends Visitor { this.addIdentifier( firstArg.value, "import", - isImport === "dynamicImport" + false, + true, + true ); } this.addDependency(firstArg.value, secondArg.properties.map(({key, value}) => { @@ -283,7 +286,9 @@ const importedIdentifierVisitor = new (class extends Visitor { this.addIdentifier( firstArg.value, "import", - isImport === "dynamicImport" + false, + false, + false ); const importIdentifiers = secondArg.properties.map(({key}) => key.name || "*"); // console.log(importIdentifiers); @@ -321,8 +326,8 @@ const importedIdentifierVisitor = new (class extends Visitor { } }); -export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = new Set(), resolveMap, fileImportState, arch) { - const possibleIndexes = findPossibleIndexes(source, [ +export function removeUnusedExports(file, exportInfo, allFilesOnBundle = new Set(), resolveMap, fileImportState, arch) { + const possibleIndexes = findPossibleIndexes(file.dataString, [ "export", "exportDefault", "link", @@ -333,10 +338,18 @@ export function removeUnusedExports(source, hash, exportInfo, allFilesOnBundle = return {}; } - const ast = tryToParse(source, hash); - removeUnusedExportsVisitor.visit(ast, source, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, fileImportState, arch); - const newSource = generate(ast, getDefaultOptions()).code; - return {source: newSource, madeChanges: removeUnusedExportsVisitor.madeChanges}; + const ast = tryToParse(file.dataString, file.hash); + removeUnusedExportsVisitor.visit(ast, file.dataString, possibleIndexes, exportInfo, allFilesOnBundle, resolveMap, fileImportState, arch); + const babelOptions = getDefaultOptions({ + nodeMajorVersion: parseInt(process.versions.node, 10), + compileForShell: true + }); + + const filename = file.absModuleId; + console.log(filename); + const {code, map} = generate(ast, {...babelOptions, sourceMaps: true, sourceFileName: filename}); + + return {source: code, map, madeChanges: removeUnusedExportsVisitor.madeChanges}; } const removeUnusedExportsVisitor = new (class extends Visitor { @@ -411,23 +424,24 @@ const removeUnusedExportsVisitor = new (class extends Visitor { if (isImport) { if(firstArg.value && firstArg.value.startsWith("meteor/")) return; if(node.arguments.length <= 1) return; + if(Resolver.isNative(firstArg.value)){ + return; + } const absPath = this.resolveMap.get(firstArg.value); const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; + const importStatus = this.fileImportState.get(absPath); + const isDynamic = importStatus === 'dynamic'; if(fileIsInBundle && isPropertyWithName(node.callee.property, "link")){ // we are seeing a static import, but this import might have been "tree-shaked", remaining only the dynamic import - const importStatus = this.fileImportState.get(absPath); - if(importStatus === 'dynamic' || !importStatus){ + if(isDynamic || !importStatus){ path.replace({ type: "BooleanLiteral", value: false, }); } } - if (!fileIsInBundle) { + if (!fileIsInBundle && !isDynamic) { // we don't want to remove any native node module import, as they are not bundled in the server bundle - if(Resolver.isNative(firstArg.value)){ - return; - } path.replace({ type: "BooleanLiteral", value: false, diff --git a/tools/isobuild/linker.js b/tools/isobuild/linker.js index f061daeeba8..fa6c03897c7 100644 --- a/tools/isobuild/linker.js +++ b/tools/isobuild/linker.js @@ -285,6 +285,7 @@ _.extend(Module.prototype, { }); const stubArray = file.deps.slice(0); + console.log(`file.deps: ${file.deps}`); if (file.absModuleId.endsWith("/package.json") && file.jsonData) { @@ -746,6 +747,9 @@ const getPrelinkedOutputCached = require("optimism").wrap( map: file.sourceMap || null, }; + if(file.absModuleId && file.absModuleId.includes("acorn")){ + debugger; + } var chunks = []; var pathNoSlash = convertColons(file.servePath.replace(/^\//, "")); From eab13c8069b233d141c439d0b9cb5975406c7513 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 15 Oct 2020 11:34:13 -0300 Subject: [PATCH 24/35] Fix missing imports from importMap, reenable tree shaking. --- tools/isobuild/bundler.js | 13 ++++++------- tools/isobuild/import-scanner.ts | 7 +------ tools/isobuild/js-analyze.js | 11 +++++------ tools/isobuild/linker.js | 1 - 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index bb7982c9a2c..a482feab2de 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1175,13 +1175,15 @@ class Target { const fileResolveMap = resolveMap.get(file.absPath) || new Map(); Object.entries(file.deps || {}).forEach(([key, depInfo]) => { const absKey = fileResolveMap.get(key); - const wasImported = importMap.get(absKey) || false; - importMap.set(absKey, {commonJsImported: wasImported || depInfo.commonJsImported}); + const importMapValue = importMap.get(absKey); + const wasImported = importMapValue && importMapValue.commonJsImported || false; + importMap.set(absKey, {...importMapValue, commonJsImported: wasImported || depInfo.commonJsImported}); }) Object.entries(file.imports || []).forEach(([key, object]) => { + const importMapValue = importMap.get(key) || {}; importMap.set(key, { - ...importMap.get(key) || {}, - deps: [...new Set([...(importMap.get(key)?.deps || []), ...object.deps])], + ...importMapValue || [], + deps: [...new Set([...(importMapValue?.deps || []), ...object.deps])], sideEffects: object.sideEffects }) }) @@ -1219,9 +1221,6 @@ class Target { if(importedSymbolsFromFile?.commonJsImported){ return; } - if(file.absPath && file.absPath.includes("node_modules/acorn/dist/acorn.mjs")){ - debugger; - } const { source: newSource, map, madeChanges } = removeUnusedExports( file, importedSymbolsFromFile, diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 6cb0647b007..19c33d93bc9 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -1261,9 +1261,6 @@ export default class ImportScanner { } throw e; } - if(file.absModuleId?.includes("console")){ - debugger; - } const rootChildren : ImportTreeNode = { @@ -1310,7 +1307,7 @@ export default class ImportScanner { const visitFrom = `${fromFilePath}->${absImportedPath}`; - const depHasSideEffects = info.sideEffects || hasSideEffects; + const depHasSideEffects = info.sideEffects; if (depFile) { // We should never have stored a fake file in this.outputFiles, so @@ -1926,7 +1923,6 @@ export default class ImportScanner { const deps = pkgFile.deps || (pkgFile.deps = Object.create(null)); const absPkgJsonPath = pathJoin(this.sourceRoot, pkgFile.sourcePath); - console.log(`resolvePkgJsonBrowserAliases: ${JSON.stringify(browser)}`); Object.keys(browser).forEach(sourceId => { deps[sourceId] = deps[sourceId] || {}; @@ -1944,7 +1940,6 @@ export default class ImportScanner { } const sourceAbsModuleId = this.getAbsModuleId(source.path); - console.log(`sourceAbsModuleId: ${sourceAbsModuleId}`); if(sourceAbsModuleId === '/node_modules/meteor-node-stubs/node_modules/assert/node_modules/util/support/isBuffer.js'){ debugger; } diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index c87f9669410..ff78c5d2552 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -211,14 +211,14 @@ const importedIdentifierVisitor = new (class extends Visitor { } if (isIdWithName(node.callee, "require")) { - this.addIdentifier(firstArg.value, "require", false, true, true); - this.addDependency(firstArg.value, ['*'], true); + this.addIdentifier(firstArg.value, "require", false, true, false); + this.addDependency(firstArg.value, ['*'], false); } else if (node.callee.type === "Import" || isIdWithName(node.callee, "import") || isPropertyWithName(node.callee.property, "dynamicImport") ) { - this.addIdentifier(firstArg.value, "import", true, true, true); - this.addDependency(firstArg.value, ['*'], true); + this.addIdentifier(firstArg.value, "import", true, true, false); + this.addDependency(firstArg.value, ['*'], false); } else { if (isModuleUsage) { const isImport = @@ -270,7 +270,7 @@ const importedIdentifierVisitor = new (class extends Visitor { "import", false, true, - true + false ); } this.addDependency(firstArg.value, secondArg.properties.map(({key, value}) => { @@ -346,7 +346,6 @@ export function removeUnusedExports(file, exportInfo, allFilesOnBundle = new Set }); const filename = file.absModuleId; - console.log(filename); const {code, map} = generate(ast, {...babelOptions, sourceMaps: true, sourceFileName: filename}); return {source: code, map, madeChanges: removeUnusedExportsVisitor.madeChanges}; diff --git a/tools/isobuild/linker.js b/tools/isobuild/linker.js index fa6c03897c7..03b53526df8 100644 --- a/tools/isobuild/linker.js +++ b/tools/isobuild/linker.js @@ -285,7 +285,6 @@ _.extend(Module.prototype, { }); const stubArray = file.deps.slice(0); - console.log(`file.deps: ${file.deps}`); if (file.absModuleId.endsWith("/package.json") && file.jsonData) { From ed04c46a530ada963421b39bb6a1a7cfe4ca5add Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 15 Oct 2020 11:47:30 -0300 Subject: [PATCH 25/35] Fix removal of legit imports on dynamic info. Only remove imports of files that are not in the bundle anymore. --- tools/isobuild/import-scanner.ts | 6 +++--- tools/isobuild/js-analyze.js | 13 +------------ 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 19c33d93bc9..c4cf035794b 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -429,7 +429,7 @@ export default class ImportScanner { private filesInfo: Map; - private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo, forDynamicImport: boolean): AnalyzeInfo { + private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo): AnalyzeInfo { /* dependencies?: Record; proxyDependencies?: Record; @@ -1245,10 +1245,10 @@ export default class ImportScanner { // this is actually an temporary dependency tree, we will need to analyze the // children to see what they are exporting const cachedFileInfo = this.filesInfo.get(file.absPath) || {}; - mergedInfo = this.mergeImportInfo(cachedFileInfo, importInfo, forDynamicImport); + mergedInfo = this.mergeImportInfo(cachedFileInfo, importInfo); // we only need the local view here, later we will need the global, merged one this.filesInfo.set(file.absPath, mergedInfo); - this.mergeImportInfo(file, importInfo, forDynamicImport) + this.mergeImportInfo(file, importInfo) } catch (e) { if (e.$ParseError) { diff --git a/tools/isobuild/js-analyze.js b/tools/isobuild/js-analyze.js index ff78c5d2552..e1aff40ed60 100644 --- a/tools/isobuild/js-analyze.js +++ b/tools/isobuild/js-analyze.js @@ -428,18 +428,7 @@ const removeUnusedExportsVisitor = new (class extends Visitor { } const absPath = this.resolveMap.get(firstArg.value); const fileIsInBundle = absPath && this.allFilesOnBundle.has(absPath) || false; - const importStatus = this.fileImportState.get(absPath); - const isDynamic = importStatus === 'dynamic'; - if(fileIsInBundle && isPropertyWithName(node.callee.property, "link")){ - // we are seeing a static import, but this import might have been "tree-shaked", remaining only the dynamic import - if(isDynamic || !importStatus){ - path.replace({ - type: "BooleanLiteral", - value: false, - }); - } - } - if (!fileIsInBundle && !isDynamic) { + if (!fileIsInBundle) { // we don't want to remove any native node module import, as they are not bundled in the server bundle path.replace({ type: "BooleanLiteral", From e40605519fc9e18247cbd89d6510504563f06019 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 15 Oct 2020 12:39:19 -0300 Subject: [PATCH 26/35] Removing console log --- tools/isobuild/bundler.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index a482feab2de..b9619e24c1c 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1200,8 +1200,6 @@ class Target { }); const appSideEffects = sourceBatch ? sourceBatch.unibuild.pkg.sideEffects : true; - console.log(`appSideEffects: ${appSideEffects}`); - // now we need to remove the exports, and let the minifier do it's job later sourceBatches.forEach((sourceBatch) => { const unibuild = sourceBatch.unibuild; From 59aef3f73c988af5f121d70114bee60c60893231 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Fri, 16 Oct 2020 15:16:14 -0300 Subject: [PATCH 27/35] Fix missing imports - global variable is not always populated. --- tools/isobuild/import-scanner.ts | 14 ++++---------- .../imports/links/jsx-import-test/parent.js | 2 +- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index c4cf035794b..aece92867da 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -845,10 +845,8 @@ export default class ImportScanner { if(!root.status) root.status = ImportTreeNodeStatus.WHITE; const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} if(root.absImportedPath) { - if(root?.absImportedPath?.includes("uuid")){ - debugger; - } - this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo, {imported: this.importedStatusFile[root.absImportedPath]})); + const imported = this.importedStatusFile[root.absImportedPath] || root.depFile?.imported || false; + this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo, {imported})); } root.status = ImportTreeNodeStatus.GRAY; @@ -1231,9 +1229,6 @@ export default class ImportScanner { let importInfo; let mergedInfo : AnalyzeInfo; - if(file.absPath?.includes("@material-ui/icons/esm/index.js")){ - debugger - } try { importInfo = this.findImportedModuleIdentifiers( @@ -1386,6 +1381,7 @@ export default class ImportScanner { } const hasAnyImports = !!(file.imports || {})[absImportedPath]; + if(file.proxyImports && file.proxyImports[id] && parentImportSymbols && @@ -1857,9 +1853,7 @@ export default class ImportScanner { private setImportedStatusGlobal(file: File, status: string | boolean){ const newStatus = setImportedStatus(file, status); - if(isHigherStatus(newStatus, this.importedStatusFile[file.absPath])){ - this.importedStatusFile[file.absPath] = newStatus; - } + this.importedStatusFile[file.absPath] = newStatus } private addPkgJsonToOutput( diff --git a/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js b/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js index ba2d46a1eb0..3478298bd0e 100644 --- a/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js +++ b/tools/tests/apps/modules/imports/links/jsx-import-test/parent.js @@ -1 +1 @@ -export { default as jsx } from "./child"; +export jsx from "./child"; From 70b2b02c5751b21ed423c473cf5e9896085283fc Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Tue, 20 Oct 2020 15:12:34 -0300 Subject: [PATCH 28/35] Fix dynamic/static import status. --- tools/isobuild/import-scanner.ts | 3 +-- tools/tests/apps/dynamic-import/package-lock.json | 6 +++--- tools/tests/apps/dynamic-import/tests.js | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index aece92867da..2d8312d31e6 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -845,8 +845,7 @@ export default class ImportScanner { if(!root.status) root.status = ImportTreeNodeStatus.WHITE; const importInfo = root.absImportedPath && this.filesInfo.get(root.absImportedPath) || {} if(root.absImportedPath) { - const imported = this.importedStatusFile[root.absImportedPath] || root.depFile?.imported || false; - this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo, {imported})); + this.addFile(root.absImportedPath, Object.assign(root.depFile, importInfo)); } root.status = ImportTreeNodeStatus.GRAY; diff --git a/tools/tests/apps/dynamic-import/package-lock.json b/tools/tests/apps/dynamic-import/package-lock.json index dc4ac173b7f..5c197127095 100644 --- a/tools/tests/apps/dynamic-import/package-lock.json +++ b/tools/tests/apps/dynamic-import/package-lock.json @@ -196,9 +196,9 @@ } }, "meteor-node-stubs": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/meteor-node-stubs/-/meteor-node-stubs-1.0.0.tgz", - "integrity": "sha512-QJwyv23wyXD3uEMzk5Xr/y5ezoVlCbHvBbrgdkVadn84dmifLRbs0PtD6EeNw5NLIk+SQSfxld7IMdEsneGz5w==", + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/meteor-node-stubs/-/meteor-node-stubs-1.0.1.tgz", + "integrity": "sha512-I4PE/z7eAl45XEsebHA4pcQbgjqEdK3EBGgiUoIZBi3bMQcMq6blLWZo+WdtK4Or9X4NJOiYWw4GmHiubr3egA==", "requires": { "assert": "^1.4.1", "browserify-zlib": "^0.2.0", diff --git a/tools/tests/apps/dynamic-import/tests.js b/tools/tests/apps/dynamic-import/tests.js index 3dd68d24eef..4a9d1b1d194 100644 --- a/tools/tests/apps/dynamic-import/tests.js +++ b/tools/tests/apps/dynamic-import/tests.js @@ -57,9 +57,9 @@ describe("dynamic import(...)", function () { it("static package.json, static package", function () { import { name } from "acorn/package.json"; - import acorn from "acorn"; + import { parse } from "acorn"; assert.strictEqual(name, "acorn"); - assert.strictEqual(typeof acorn.parse, "function"); + assert.strictEqual(typeof parse, "function"); }); it("static package.json, dynamic package", function () { From 2e5a99892c16aaeea07036aa6a17a486fb3a41e7 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Wed, 21 Oct 2020 17:04:50 -0300 Subject: [PATCH 29/35] Fix missing modules error. --- tools/isobuild/bundler.js | 25 ++++++++++++++++++++----- tools/isobuild/compiler-plugin.js | 9 ++++++--- tools/isobuild/import-scanner.ts | 7 +++++++ 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index b9619e24c1c..30d3a0f8d69 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1136,8 +1136,12 @@ class Target { const isWeb = archinfo.matches(this.arch, 'web'); const isOs = archinfo.matches(this.arch, 'os'); - const { jsOutputFilesMap, resolveMap = new Map(), fileImportState = new Map() } = compilerPluginModule.PackageSourceBatch - .computeJsOutputFilesMap(sourceBatches); + const { + jsOutputFilesMap, + resolveMap = new Map(), + fileImportState = new Map(), + allMissingModulesAbsPath = {} + } = compilerPluginModule.PackageSourceBatch.computeJsOutputFilesMap(sourceBatches); sourceBatches.forEach(batch => { const { unibuild } = batch; @@ -1175,13 +1179,24 @@ class Target { const fileResolveMap = resolveMap.get(file.absPath) || new Map(); Object.entries(file.deps || {}).forEach(([key, depInfo]) => { const absKey = fileResolveMap.get(key); - const importMapValue = importMap.get(absKey); + const importMapValue = importMap.get(absKey) || + (allMissingModulesAbsPath && allMissingModulesAbsPath[key] ? + importMap.get(allMissingModulesAbsPath[key]) : {}); const wasImported = importMapValue && importMapValue.commonJsImported || false; importMap.set(absKey, {...importMapValue, commonJsImported: wasImported || depInfo.commonJsImported}); }) Object.entries(file.imports || []).forEach(([key, object]) => { - const importMapValue = importMap.get(key) || {}; - importMap.set(key, { + let importMapValue; + let identifier; + if(files.pathIsAbsolute(key)){ + importMapValue = importMap.get(key) || {}; + identifier = key; + }else{ + importMapValue = allMissingModulesAbsPath && allMissingModulesAbsPath[key] ? + importMap.get(allMissingModulesAbsPath[key]) : {}; + identifier = allMissingModulesAbsPath[key] || key; + } + importMap.set(identifier, { ...importMapValue || [], deps: [...new Set([...(importMapValue?.deps || []), ...object.deps])], sideEffects: object.sideEffects diff --git a/tools/isobuild/compiler-plugin.js b/tools/isobuild/compiler-plugin.js index 986e1906ae2..f6583319c19 100644 --- a/tools/isobuild/compiler-plugin.js +++ b/tools/isobuild/compiler-plugin.js @@ -1326,6 +1326,7 @@ export class PackageSourceBatch { // Records the subset of allMissingModules that were successfully // relocated to a source batch that could handle them. const allRelocatedModules = Object.create(null); + let allMissingModulesAbsPath = Object.create(null); const scannerMap = new Map; const sourceBatch = sourceBatches.find((sourceBatch) => { const name = sourceBatch.unibuild.pkg.name || null; @@ -1432,6 +1433,7 @@ export class PackageSourceBatch { scannerMap.get(name).scanMissingModules(missing); ImportScanner.mergeMissing(allRelocatedModules, newlyAdded); ImportScanner.mergeMissing(nextMissingModules, newlyMissing); + allMissingModulesAbsPath = { ...scannerMap.get(name).missingDepsAbsPath, ...(allMissingModulesAbsPath || {})}; }); if (! _.isEmpty(nextMissingModules)) { @@ -1516,10 +1518,10 @@ export class PackageSourceBatch { resolveMap.set(key, current); }); }); - return this._watchOutputFiles(map, resolveMap, fileImportState); + return this._watchOutputFiles(map, resolveMap, fileImportState, allMissingModulesAbsPath); } - static _watchOutputFiles(jsOutputFilesMap, resolveMap, fileImportState) { + static _watchOutputFiles(jsOutputFilesMap, resolveMap, fileImportState, allMissingModulesAbsPath) { // Watch all output files produced by computeJsOutputFilesMap. jsOutputFilesMap.forEach(entry => { entry.files.forEach(file => { @@ -1546,7 +1548,8 @@ export class PackageSourceBatch { } }); }); - return { jsOutputFilesMap, resolveMap, fileImportState }; + + return { jsOutputFilesMap, resolveMap, fileImportState, allMissingModulesAbsPath }; } static _warnAboutMissingModules(missingModules) { diff --git a/tools/isobuild/import-scanner.ts b/tools/isobuild/import-scanner.ts index 2d8312d31e6..1254e4d49c7 100644 --- a/tools/isobuild/import-scanner.ts +++ b/tools/isobuild/import-scanner.ts @@ -427,6 +427,7 @@ export default class ImportScanner { private allMissingModules: MissingMap = Object.create(null); private outputFiles: File[] = []; private filesInfo: Map; + public missingDepsAbsPath: Record; private mergeImportInfo(file: any, importInfo: JSAnalyzeInfo): AnalyzeInfo { @@ -501,6 +502,7 @@ export default class ImportScanner { this.sourceRoot = sourceRoot; this.nodeModulesPaths = nodeModulesPaths; this.visitedFromRoots = {}; + this.missingDepsAbsPath = {}; this.defaultHandlers = new DefaultHandlers({ cacheDir, @@ -1281,6 +1283,11 @@ export default class ImportScanner { if (! absImportedPath) { return; } + // we are dealing with a dep that was marked as missing + if(file[fakeSymbol]){ + this.missingDepsAbsPath[id] = absImportedPath; + } + if(file.imports && file.imports[id]){ file.imports[absImportedPath] = file.imports[id]; if(mergedInfo && mergedInfo.imports){ From f374f157391519aa8aedc31d11149b2003e9d07f Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Wed, 21 Oct 2020 17:38:32 -0300 Subject: [PATCH 30/35] Remove side effects free from base64. EJSON.newBinary is not a function. --- packages/base64/package.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/base64/package.js b/packages/base64/package.js index 061af890602..547992b2a4c 100644 --- a/packages/base64/package.js +++ b/packages/base64/package.js @@ -6,12 +6,10 @@ Package.describe({ Package.onUse(api => { api.export('Base64'); api.use('ecmascript'); - api.setSideEffects(false); api.mainModule('base64.js'); }); Package.onTest(api => { api.use(['ecmascript', 'tinytest', 'ejson']); - api.setSideEffects(false); api.addFiles('base64_test.js', ['client', 'server']); }); From 85e129562f73b018bd27bd201122e354021591e0 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Wed, 21 Oct 2020 18:14:34 -0300 Subject: [PATCH 31/35] Remove side effects free from base64. EJSON.newBinary is not a function. --- packages/ejson/ejson.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ejson/ejson.js b/packages/ejson/ejson.js index 7e4ff25dd8f..a4670297d28 100644 --- a/packages/ejson/ejson.js +++ b/packages/ejson/ejson.js @@ -9,7 +9,7 @@ import { isInfOrNaN, handleError, } from './utils'; -import Base64 from 'meteor/base64'; + /** * @namespace * @summary Namespace for EJSON functions From c327898ec3237fc25a4d5febb4548d4ee93e41df Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 22 Oct 2020 10:51:22 -0300 Subject: [PATCH 32/35] Get dead code elimination back --- packages/minifier-js/minifier.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index 628b8da6877..7be44d97cf5 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -48,9 +48,8 @@ meteorJsMinify = function (source, options) { }, {}); globalDefsMapping.Meteor = {...globalDefsMapping.Meteor, ...customSettings}; try { - //TODO: uncomment this code - // var optimizedCode = Babel.replaceMeteorInternalState(source, globalDefsMapping) - var terserResult = Meteor.wrapAsync(callback => terser.minify(source, { + var optimizedCode = Babel.replaceMeteorInternalState(source, globalDefsMapping) + var terserResult = Meteor.wrapAsync(callback => terser.minify(optimizedCode, { compress: { drop_debugger: false, unused: true, From 3cd86645af328743f4fb42c72343599a8e6249b0 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Thu, 22 Oct 2020 18:03:50 -0300 Subject: [PATCH 33/35] Fix dead code elimination error --- packages/babel-compiler/babel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-compiler/babel.js b/packages/babel-compiler/babel.js index a2eb6462662..0ddce510a78 100644 --- a/packages/babel-compiler/babel.js +++ b/packages/babel-compiler/babel.js @@ -52,7 +52,7 @@ Babel = { try { const globalDefsKeys = Object.keys(globalDefsMapping); return Npm.require("@babel/core").transformSync(source, { - ...getMeteorBabel().getMinifierOptions(), + ...getMeteorBabel().getDefaultOptions(), plugins: [ function replaceStateVars({types: t}) { return { From 4613a5a57eb78c4b1f73e42408dd127f493810f9 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Fri, 23 Oct 2020 12:12:16 -0300 Subject: [PATCH 34/35] Use buildMode for deciding if isTest or isProduction --- packages/babel-compiler/babel.js | 2 +- packages/minifier-js/minifier.js | 6 +++--- tools/isobuild/bundler.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/babel-compiler/babel.js b/packages/babel-compiler/babel.js index 0ddce510a78..401c53a3e0f 100644 --- a/packages/babel-compiler/babel.js +++ b/packages/babel-compiler/babel.js @@ -52,7 +52,7 @@ Babel = { try { const globalDefsKeys = Object.keys(globalDefsMapping); return Npm.require("@babel/core").transformSync(source, { - ...getMeteorBabel().getDefaultOptions(), + compact: false, plugins: [ function replaceStateVars({types: t}) { return { diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index 7be44d97cf5..6d6619b8459 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -1,11 +1,11 @@ var terser; -const getGlobalDefsOptions = ({ arch }) => ({ +const getGlobalDefsOptions = ({ arch, buildMode }) => ({ "Meteor.isServer": false, - "Meteor.isTest": false, + "Meteor.isTest": buildMode === "test", "Meteor.isDevelopment": false, "Meteor.isClient": true, - "Meteor.isProduction": true, + "Meteor.isProduction": buildMode !== "test", "Meteor.isCordova": arch === 'web.cordova', }); diff --git a/tools/isobuild/bundler.js b/tools/isobuild/bundler.js index 30d3a0f8d69..1ba120acdbb 100644 --- a/tools/isobuild/bundler.js +++ b/tools/isobuild/bundler.js @@ -1437,7 +1437,7 @@ class Target { minifyJs(minifierDef, minifyMode) { const staticFiles = []; const dynamicFiles = []; - const { arch } = this; + const { arch, buildMode } = this; const inputHashesByJsFile = new Map; this.js.forEach(file => { @@ -1460,7 +1460,7 @@ class Target { buildmessage.enterJob('minifying app code', function () { try { Promise.all([ - markedMinifier(staticFiles, { minifyMode, arch }), + markedMinifier(staticFiles, { minifyMode, arch, buildMode }), ...dynamicFiles.map( file => markedMinifier([file], { minifyMode, arch }) ), From 858a0452c81fe75c8fd8b0bdbffe493557f65039 Mon Sep 17 00:00:00 2001 From: Renan Castro Date: Fri, 23 Oct 2020 12:37:45 -0300 Subject: [PATCH 35/35] Use buildMode for deciding if isTest or isProduction --- packages/minifier-js/minifier.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/minifier-js/minifier.js b/packages/minifier-js/minifier.js index 6d6619b8459..91ab69b27da 100644 --- a/packages/minifier-js/minifier.js +++ b/packages/minifier-js/minifier.js @@ -5,7 +5,7 @@ const getGlobalDefsOptions = ({ arch, buildMode }) => ({ "Meteor.isTest": buildMode === "test", "Meteor.isDevelopment": false, "Meteor.isClient": true, - "Meteor.isProduction": buildMode !== "test", + "Meteor.isProduction": true, "Meteor.isCordova": arch === 'web.cordova', });