Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

fix: Added a build:dev yarn task to PVA Publish to create source maps #4831

Merged
a-b-r-o-w-n merged 10 commits into from
Nov 17, 2020
Merged

fix: Added a build:dev yarn task to PVA Publish to create source maps #4831

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1f9ecc6
Added a build:dev yarn task to create working node-side sourcemaps
tonyanziano Nov 16, 2020
6acb8a0
Added missing newline
tonyanziano Nov 16, 2020
3968f22
Merge branch 'main' into toanzian/fix/pva-sourcemaps
a-b-r-o-w-n Nov 16, 2020
7784794
Fixed up dev config
tonyanziano Nov 16, 2020
6a1ae6c
Merge branch 'toanzian/fix/pva-sourcemaps' of https://github.com/micr…
tonyanziano Nov 16, 2020
a875fb0
Merge branch 'main' into toanzian/fix/pva-sourcemaps
tonyanziano Nov 16, 2020
79e4dd3
Merge branch 'main' into toanzian/fix/pva-sourcemaps
tonyanziano Nov 17, 2020
5a0e6f1
Merge branch 'main' into toanzian/fix/pva-sourcemaps
hatpick Nov 17, 2020
e37572e
Merge branch 'main' into toanzian/fix/pva-sourcemaps
tonyanziano Nov 17, 2020
2d2dafc
Merge branch 'main' into toanzian/fix/pva-sourcemaps
tonyanziano Nov 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/launch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@
"${workspaceRoot}/Composer/packages/electron-server/build/**/*.js",
"${workspaceRoot}/Composer/packages/server/build/**/*.js",
"${workspaceRoot}/extensions/**/*.js"
]
],
"sourceMaps": true
},
{
"name": "Debug current jest test",
Expand Down
6 changes: 4 additions & 2 deletions extensions/pvaPublish/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"name": "pva-publish-composer",
"version": "1.0.0",
"scripts": {
"build": "yarn clean && webpack",
"build": "yarn clean && cross-env NODE_ENV=production webpack",
"build:dev": "yarn clean && cross-env NODE_ENV=development webpack",
"clean": "rimraf lib",
"test": "yarn test:node && yarn test:ui",
"test:node": "jest --config jest.config.node.js",
Expand All @@ -28,9 +29,9 @@
},
"main": "lib/node/index.js",
"dependencies": {
"@botframework-composer/types": "file:../../Composer/packages/types",
"@bfc/extension": "file:../../Composer/packages/extension",
"@bfc/extension-client": "file:../../Composer/packages/extension-client",
"@botframework-composer/types": "file:../../Composer/packages/types",
"@uifabric/icons": "7.3.59",
"@uifabric/react-hooks": "7.4.12",
"debug": "^4.2.0",
Expand All @@ -47,6 +48,7 @@
"@botframework-composer/test-utils": "^0.0.1",
"@types/jest": "^26.0.15",
"@types/node": "^14.11.2",
"cross-env": "^7.0.2",
"jest": "^26.6.0",
"mini-svg-data-uri": "^1.2.3",
"rimraf": "^3.0.2",
Expand Down
3 changes: 2 additions & 1 deletion extensions/pvaPublish/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
Comment thread
a-b-r-o-w-n marked this conversation as resolved.
"compilerOptions": {
"esModuleInterop": true,
"jsx": "react"
"jsx": "react",
"sourceMap": true
},
"include": [
"src/**/*",
Expand Down
108 changes: 60 additions & 48 deletions extensions/pvaPublish/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,54 +1,66 @@
const { resolve } = require('path');
const svgToMiniDataURI = require('mini-svg-data-uri');

module.exports = [
{
entry: './src/ui/index.tsx',
mode: 'production',
output: {
filename: 'publish-bundle.js',
path: resolve(__dirname, 'lib', 'ui'),
},
externals: {
// expect react & react-dom to be available in the extension host iframe
react: 'React',
'react-dom': 'ReactDOM',
},
module: {
rules: [
{
test: /\.tsx?$/,
loader: 'ts-loader',
},
{
test: /\.svg$/i,
loader: 'url-loader',
options: {
generator: (content) => svgToMiniDataURI(content.toString()),
function getWebpackConfigs() {
const isDevelopment = process.env.NODE_ENV === 'development';
console.log(`[pvaPublish] Generating ${isDevelopment ? 'development' : 'production'} bundles.`);

return [
{
entry: './src/ui/index.tsx',
mode: isDevelopment ? 'development' : 'production',
devtool: isDevelopment ? 'source-map' : undefined,
output: {
filename: 'publish-bundle.js',
path: resolve(__dirname, 'lib', 'ui'),
},
externals: {
// expect react & react-dom to be available in the extension host iframe
react: 'React',
'react-dom': 'ReactDOM',
},
module: {
rules: [
{
test: /\.tsx?$/,
loader: 'ts-loader',
},
},
],
},
resolve: {
extensions: ['.js', '.ts', '.tsx'],
},
plugins: [],
},
{
entry: './src/node/index.ts',
mode: 'production',
devtool: 'source-map',
target: 'node',
output: {
path: resolve(__dirname, 'lib', 'node'),
filename: 'index.js',
libraryTarget: 'commonjs2',
},
module: {
rules: [{ test: /\.tsx?$/, use: 'ts-loader', exclude: [/node_modules/] }],
{
test: /\.svg$/i,
loader: 'url-loader',
options: {
generator: (content) => svgToMiniDataURI(content.toString()),
},
},
],
},
resolve: {
extensions: ['.js', '.ts', '.tsx'],
},
plugins: [],
},
resolve: {
extensions: ['.js', '.ts', '.tsx'],
{
entry: './src/node/index.ts',
mode: isDevelopment ? 'development' : 'production',
target: 'node',
devtool: isDevelopment ? 'source-map' : undefined,

@a-b-r-o-w-n a-b-r-o-w-n Nov 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be the other way around?

@tonyanziano tonyanziano Nov 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I want it to behave like this:

if (isDevelopment)
  // generate source maps
else 
  // don't generate source maps

@a-b-r-o-w-n a-b-r-o-w-n Nov 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

interesting. it's usually the other way around. until we start reporting errors, I guess it doesn't matter too much.

@tonyanziano tonyanziano Nov 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would think you would want source maps when developing so you can step through your code and work out any bugs, and then you would not want them in production to reduce size and because you aren't often debugging the production code.

We also don't copy the source files of the extensions into the production app so the source maps wouldn't even work would they?

@a-b-r-o-w-n a-b-r-o-w-n Nov 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Normally, you would use something like cheap-eval-source-map for dev because of build speed. In this case, we aren't concerned as much with build speed.

output: {
path: resolve(__dirname, 'lib', 'node'),
filename: 'index.js',
libraryTarget: 'commonjs2',
/**
* Node files aren't being loaded by Webpack, so we want the source maps to point to the files on disk
*/
devtoolModuleFilenameTemplate: isDevelopment ? '[absolute-resource-path]' : undefined,
},
module: {
rules: [{ test: /\.tsx?$/, use: 'ts-loader', exclude: [/node_modules/] }],
},
resolve: {
extensions: ['.js', '.ts', '.tsx'],
},
},
},
];
];
}

module.exports = getWebpackConfigs();
9 changes: 8 additions & 1 deletion extensions/pvaPublish/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2659,6 +2659,13 @@ create-hmac@^1.1.0, create-hmac@^1.1.4, create-hmac@^1.1.7:
safe-buffer "^5.0.1"
sha.js "^2.4.8"

cross-env@^7.0.2:
version "7.0.2"
resolved "https://registry.yarnpkg.com/cross-env/-/cross-env-7.0.2.tgz#bd5ed31339a93a3418ac4f3ca9ca3403082ae5f9"
integrity sha512-KZP/bMEOJEDCkDQAyRhu3RL2ZO/SUVrxQVI0G3YEQ+OLbRA3c6zgixe8Mq8a/z7+HKlNEjo8oiLUs8iRijY2Rw==
dependencies:
cross-spawn "^7.0.1"

cross-spawn@^6.0.0, cross-spawn@^6.0.5:
version "6.0.5"
resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4"
Expand All @@ -2670,7 +2677,7 @@ cross-spawn@^6.0.0, cross-spawn@^6.0.5:
shebang-command "^1.2.0"
which "^1.2.9"

cross-spawn@^7.0.0:
cross-spawn@^7.0.0, cross-spawn@^7.0.1:
version "7.0.3"
resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-7.0.3.tgz#f73a85b9d5d41d045551c177e2882d4ac85728a6"
integrity sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w==
Expand Down