-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Storage] Rollup for tests in storage #2297
Changes from all commits
5663490
0fb1846
558cac6
e6803cb
9042ef6
9471ef0
44570b2
e72f5cd
154af83
66a5fed
6d2fc3f
6363eb4
93e42fc
7bddadf
0a311f9
4fecbcc
4eba327
3665dd4
9921461
606fe5c
7899e4c
ed5fc86
fb5970d
ac390da
ce2e0b6
6ce31db
e9d95fc
d9c9456
9ac4f0f
34e9fc5
7d2b4ba
3237181
7788fd0
0e2d10d
cf6524c
518a1f0
3dd97f2
fb50e69
ea3b017
21f1302
10b71ee
2d2c526
312b42b
5a630c7
c7f913e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,13 @@ const zip = require("gulp-zip"); | |
const version = require("./package.json").version; | ||
const zipFileName = `azurestoragejs.blob-${version}.zip`; | ||
|
||
gulp.task("zip", function(callback) { | ||
gulp.task("zip", function (callback) { | ||
gulp | ||
.src(["browser/azure-storage.blob.js", "browser/azure-storage.blob.min.js", "browser/*.txt"]) | ||
.src([ | ||
"browser/azure-storage-blob.min.js", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please update section "JavaScript Bundle" in "readme.md". Same with other readme.md files for queue/file. Please check any documents points to old bundle name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing out. |
||
"browser/azure-storage-blob.js", | ||
"browser/*.txt" | ||
]) | ||
.pipe(zip(zipFileName)) | ||
.pipe(gulp.dest("browser")) | ||
.on("end", callback); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,136 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
import nodeResolve from "rollup-plugin-node-resolve"; | ||
import multiEntry from "rollup-plugin-multi-entry"; | ||
import cjs from "rollup-plugin-commonjs"; | ||
import replace from "rollup-plugin-replace"; | ||
import { uglify } from "rollup-plugin-uglify"; | ||
import sourcemaps from "rollup-plugin-sourcemaps"; | ||
import shim from "rollup-plugin-shim"; | ||
// import visualizer from "rollup-plugin-visualizer"; | ||
|
||
const version = require("./package.json").version; | ||
const banner = [ | ||
"/*!", | ||
` * Azure Storage SDK for JavaScript - Blob, ${version}`, | ||
" * Copyright (c) Microsoft and contributors. All rights reserved.", | ||
" */" | ||
].join("\n"); | ||
|
||
const pkg = require("./package.json"); | ||
const depNames = Object.keys(pkg.dependencies); | ||
const production = process.env.NODE_ENV === "production"; | ||
|
||
export function nodeConfig(test = false) { | ||
const externalNodeBuiltins = ["@azure/ms-rest-js", "crypto", "fs", "events", "os", "stream"]; | ||
const baseConfig = { | ||
input: "dist-esm/src/index.js", | ||
external: depNames.concat(externalNodeBuiltins), | ||
output: { | ||
file: "dist/index.js", | ||
format: "cjs", | ||
sourcemap: true | ||
}, | ||
preserveSymlinks: false, | ||
plugins: [ | ||
sourcemaps(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does "sourcemaps()" do? I remembered a sourcemap file is generated under dist folder without this plugin. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Can you remove "dotenv" from coverage report? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sourcemaps is for threading the sourcemap across multiple transformations. Rollup by default won't consume sourcemaps so the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Update - I tried excluding that specific file(and every possible node_modules path) by adding rules in Looks like I'm opening a new issue to investigate further - #2820 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @HarshaNalluru - I apologize. The issue 1099 that you reference was mine. It was user error. |
||
replace({ | ||
delimiters: ["", ""], | ||
values: { | ||
// replace dynamic checks with if (true) since this is for node only. | ||
// Allows rollup's dead code elimination to be more aggressive. | ||
"if (isNode)": "if (true)" | ||
} | ||
}), | ||
nodeResolve({ preferBuiltins: true }), | ||
cjs() | ||
] | ||
}; | ||
|
||
if (test) { | ||
// entry point is every test file | ||
baseConfig.input = ["dist-esm/test/*.spec.js", "dist-esm/test/node/*.spec.js"]; | ||
baseConfig.plugins.unshift(multiEntry({ exports: false })); | ||
|
||
// different output file | ||
baseConfig.output.file = "dist-test/index.node.js"; | ||
|
||
// mark assert as external | ||
baseConfig.external.push("assert", "fs", "path"); | ||
|
||
baseConfig.context = "null"; | ||
} else if (production) { | ||
baseConfig.plugins.push(uglify()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For Node.js, we released minified CJS bundle to customers. When there is an exception in SDK, stack trace is unreadable. Any good suggestions? @bterlson @jeremymeng @HarshaNalluru There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sourcemaps! See below. |
||
} | ||
|
||
return baseConfig; | ||
} | ||
|
||
export function browserConfig(test = false, production = false) { | ||
const baseConfig = { | ||
input: "dist-esm/src/index.browser.js", | ||
external: ["ms-rest-js"], | ||
output: { | ||
file: "browser/azure-storage-blob.js", | ||
banner: banner, | ||
format: "umd", | ||
name: "azblob", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a note: this will likely change to align with .net naming conventions. Expect something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't change this before next breaking change. : ) |
||
sourcemap: true | ||
}, | ||
preserveSymlinks: false, | ||
plugins: [ | ||
sourcemaps(), | ||
replace({ | ||
delimiters: ["", ""], | ||
values: { | ||
// replace dynamic checks with if (false) since this is for | ||
// browser only. Rollup's dead code elimination will remove | ||
// any code guarded by if (isNode) { ... } | ||
"if (isNode)": "if (false)" | ||
} | ||
}), | ||
// os is not used by the browser bundle, so just shim it | ||
shim({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these shims actually needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Update - shimming both of them is required. |
||
dotenv: `export function config() { }`, | ||
os: ` | ||
export const type = 1; | ||
export const release = 1; | ||
` | ||
}), | ||
nodeResolve({ | ||
mainFields: ["module", "browser"], | ||
preferBuiltins: false | ||
}), | ||
cjs({ | ||
namedExports: { | ||
events: ["EventEmitter"], | ||
assert: ["ok", "deepEqual", "equal", "fail", "deepStrictEqual", "notDeepEqual"] | ||
} | ||
}) | ||
] | ||
}; | ||
|
||
if (test) { | ||
baseConfig.input = ["dist-esm/test/*.spec.js", "dist-esm/test/browser/*.spec.js"]; | ||
baseConfig.plugins.unshift(multiEntry({ exports: false })); | ||
baseConfig.output.file = "dist-test/index.browser.js"; | ||
baseConfig.context = "null"; | ||
} else if (production) { | ||
baseConfig.output.file = "browser/azure-storage-blob.min.js"; | ||
baseConfig.plugins.push( | ||
uglify({ | ||
output: { | ||
preamble: banner | ||
} | ||
}) | ||
// Comment visualizer because it only works on Node.js 8+; Uncomment it to get bundle analysis report | ||
// visualizer({ | ||
// filename: "./statistics.html", | ||
// sourcemap: true | ||
// }) | ||
); | ||
} | ||
|
||
return baseConfig; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,95 +1,18 @@ | ||
import nodeResolve from "rollup-plugin-node-resolve"; | ||
import { uglify } from "rollup-plugin-uglify"; | ||
import replace from "rollup-plugin-replace"; | ||
import commonjs from "rollup-plugin-commonjs"; | ||
import shim from "rollup-plugin-shim"; | ||
// import visualizer from "rollup-plugin-visualizer"; | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
const version = require("./package.json").version; | ||
const banner = [ | ||
"/*!", | ||
` * Azure Storage SDK for JavaScript - Blob, ${version}`, | ||
" * Copyright (c) Microsoft and contributors. All rights reserved.", | ||
" */" | ||
].join("\n"); | ||
import * as base from "./rollup.base.config"; | ||
|
||
const nodeRollupConfigFactory = () => { | ||
return { | ||
external: ["@azure/ms-rest-js", "crypto", "fs", "events", "os", "stream"], | ||
input: "dist-esm/src/index.js", | ||
output: { | ||
file: "dist/index.js", | ||
format: "cjs", | ||
sourcemap: true | ||
}, | ||
preserveSymlinks: false, | ||
plugins: [nodeResolve(), uglify()] | ||
}; | ||
}; | ||
const inputs = []; | ||
|
||
const browserRollupConfigFactory = (isProduction) => { | ||
const browserRollupConfig = { | ||
input: "dist-esm/src/index.browser.js", | ||
output: { | ||
file: "browser/azure-storage.blob.js", | ||
banner: banner, | ||
format: "umd", | ||
name: "azblob", | ||
sourcemap: true | ||
}, | ||
preserveSymlinks: false, | ||
plugins: [ | ||
replace({ | ||
delimiters: ["", ""], | ||
values: { | ||
// replace dynamic checks with if (false) since this is for | ||
// browser only. Rollup's dead code elimination will remove | ||
// any code guarded by if (isNode) { ... } | ||
"if (isNode)": "if (false)" | ||
} | ||
}), | ||
// os is not used by the browser bundle, so just shim it | ||
shim({ | ||
dotenv: `export function config() { }`, | ||
os: ` | ||
export const type = 1; | ||
export const release = 1; | ||
` | ||
}), | ||
nodeResolve({ | ||
mainFields: ["module", "browser"], | ||
preferBuiltins: false | ||
}), | ||
commonjs({ | ||
namedExports: { | ||
events: ["EventEmitter"], | ||
assert: ["ok", "deepEqual", "equal", "fail", "deepStrictEqual", "notDeepEqual"] | ||
} | ||
}) | ||
] | ||
}; | ||
if (!process.env.ONLY_BROWSER) { | ||
inputs.push(base.nodeConfig()); | ||
} | ||
|
||
if (isProduction) { | ||
browserRollupConfig.output.file = "browser/azure-storage.blob.min.js"; | ||
browserRollupConfig.plugins.push( | ||
uglify({ | ||
output: { | ||
preamble: banner | ||
} | ||
}) | ||
// Comment visualizer because it only works on Node.js 8+; Uncomment it to get bundle analysis report | ||
// visualizer({ | ||
// filename: "./statistics.html", | ||
// sourcemap: true | ||
// }) | ||
); | ||
} | ||
// Disable this until we are ready to run rollup for the browser. | ||
if (!process.env.ONLY_NODE) { | ||
inputs.push(base.browserConfig()); | ||
inputs.push(base.browserConfig(false, true)); | ||
} | ||
|
||
return browserRollupConfig; | ||
}; | ||
|
||
export default [ | ||
browserRollupConfigFactory(false), | ||
browserRollupConfigFactory(true), | ||
nodeRollupConfigFactory() | ||
]; | ||
export default inputs; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,6 @@ | ||
import multi from "rollup-plugin-multi-entry"; | ||
import baseConfig from "./rollup.config"; | ||
import sourcemaps from "rollup-plugin-sourcemaps"; | ||
const [browser] = baseConfig; | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
browser.input = ["dist-esm/test/*.js", "dist-esm/test/browser/*.js"]; | ||
browser.output.sourcemap = "inline"; | ||
browser.output.file = "dist-test/index.browser.js"; | ||
browser.plugins.unshift(multi()); | ||
browser.plugins.unshift(sourcemaps()); | ||
browser.context = "null"; | ||
import * as base from "./rollup.base.config"; | ||
|
||
export default [browser]; | ||
export default [base.nodeConfig(true), base.browserConfig(true)]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required since nyc is updated from 13.3.0 to 14.0.0.
[Breaking Change]
https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md
https://github.com/istanbuljs/nyc#source-map-support-for-pre-instrumented-codebases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, how does coverage report look like? Do covered lines point to TS source code or compiled JS codes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Covered lines would point to the typescript source code.
Example -
Coverage output-html for (only) one test -