-
Notifications
You must be signed in to change notification settings - Fork 63
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
- Add typescript version for customFunctions (i.e. move customfunctio… #34
- Add typescript version for customFunctions (i.e. move customfunctio… #34
Conversation
…ns.js to customfunctions.ts) - Update webpack.config.js so that customfunctions.ts is built into a bundle file - Add/Remove various scripts commands package.json - Update relevant files to poijt to new bundle generated
}; | ||
} | ||
|
||
CustomFunctionMappings.add = add; |
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.
Since CustomFunctionMappings() is so new, can we add a comment around these to explain and give context?
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.
@mscharlock You make a good point that we need to provide better explanation, either in the code or in README documentation. We're going to continue to make updates to this, and I don't think it is required as part of this pull request.
index.html
Outdated
</head> | ||
|
||
<body> | ||
Thanks for trying custom functions for Excel! For more information, see |
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.
Can we also provide a list of the built in functions in this? I like that the taskpane did that previously and I think it could be helpful for users.
…cause this was causing us to fail in Excel Online because Office.Preview.startCustomFunctions is undefined Perhaps this is related to the CDN changes. After removing this, custom functions works in both Desktop and Excel Online
package.json
Outdated
"version": "1.0.0", | ||
"description": "Create Excel functions using JavaScript.", | ||
"main": "customfunctions.js", | ||
"name": "excel-functions", |
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.
The package name should stay the same as "excel-custom-functions".
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.
fixed
package.json
Outdated
"description": "Create Excel functions using JavaScript.", | ||
"main": "customfunctions.js", | ||
"name": "excel-functions", | ||
"version": "0.0.2", |
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.
The version should be a major version update to "2.0.0".
package.json
Outdated
"main": "customfunctions.js", | ||
"name": "excel-functions", | ||
"version": "0.0.2", | ||
"main": "./src/customfunctions.ts", |
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.
Please restore original npm package values: description, repository, homepage, bugs, etc.
package.json
Outdated
"sideload": "office-toolbox sideload -m manifest.xml -a excel", | ||
"start": "start npm run webpack && start npm run dev-server && npm run sideload", | ||
"dev-server": "webpack-dev-server --mode development --https --key ./certs/server.key --cert ./certs/server.crt --cacert ./certs/ca.crt --port 8081 --hotOnly", | ||
"webpack": "webpack -p --mode development --watch" |
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.
I recommend changing "webpack" to "watch", so that "npm run watch" runs the file watcher.
}; | ||
} | ||
|
||
CustomFunctionMappings.add = add; |
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.
@mscharlock You make a good point that we need to provide better explanation, either in the code or in README documentation. We're going to continue to make updates to this, and I don't think it is required as part of this pull request.
config/customfunctions.json
Outdated
"dimensionality": "scalar" | ||
} | ||
], | ||
"options": { |
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.
Remove "options" since we don't need sync to be set.
config/customfunctions.json
Outdated
"dimensionality": "scalar" | ||
} | ||
], | ||
"options": { |
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.
Remove options here too.
config/customfunctions.json
Outdated
} | ||
], | ||
"options": { | ||
"sync": false, |
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.
Remove "sync": false,
<bt:Url id="JS-URL" DefaultValue="https://localhost:3000/src/customfunctions.js" /> | ||
<bt:Url id="HTML-URL" DefaultValue="https://localhost:3000/index.html" /> | ||
<bt:Url id="JSON-URL" DefaultValue="https://localhost:8081/config/customfunctions.json" /> | ||
<bt:Url id="JS-URL" DefaultValue="https://localhost:8081/dist/win32/ship/index.win32.bundle" /> |
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.
As we've discussed, I believe this should be https://localhost:8081/index.win32.bundle
. We need to sort this out.
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.
Yes, we should get someone else to test out the current manifest and see if it works, because I am not having any luck when the bundle url is https://localhost:8081/index.win32.bundle
package.json
Outdated
"html-loader": "^0.5.5", | ||
"html-webpack-plugin": "^3.0.7", | ||
"html-webpack-plugin": "^3.2.0", | ||
"http-server": "^0.11.1", | ||
"office-addin-validator": "^1.0.1", |
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.
I don't think we need office-addin-validator.
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.
Looks like we do need it for sideloading. When I remove it, sideloading fails:
"Error: Cannot find module 'office-addin-validator'"
`npm run test` will now run tests using Mocha and Office.js to run the add-in and validate results.
commit ef018e9 Author: Courtney Owen <[email protected]> Date: Wed Jun 12 12:07:03 2019 -0700 Fix breaking merge change - Merge from Office-Addin-Taskpane introduced breaking change in test/webpack.config.js commit 40a985c Author: Courtney Owen <[email protected]> Date: Wed Jun 12 10:45:10 2019 -0700 Add step to modify manifest guid before running test - I am hoping this better ensures the test always runs with a unique manifest commit e5324c9 Author: Courtney Owen <[email protected]> Date: Tue Jun 11 12:31:44 2019 -0700 Remove unused sleep method from testHelpers commit c55aee3 Merge: f0a3672 f55432a Author: Courtney Owen <[email protected]> Date: Tue Jun 11 12:30:13 2019 -0700 Merge remote-tracking branch 'Office-Addin-Taskpane/master' into ui-tests commit f55432a Merge: ab330d6 de1bb5f Author: Courtney Owen <[email protected]> Date: Mon Jun 10 13:36:24 2019 -0700 Merge pull request OfficeDev#39 from TCourtneyOwen/minor-test-fix Minor fix to address issue when converting to single host commit de1bb5f Author: Courtney Owen <[email protected]> Date: Mon Jun 10 13:28:06 2019 -0700 Minor fix to address issue when converting to single host - Without this change, tests will fail for Excel after calling convertToSingleHost with "excel convert-test" params. commit ab330d6 Merge: cb1ebef 1a39181 Author: Adam Krantz <[email protected]> Date: Fri Jun 7 13:54:35 2019 -0700 Merge pull request OfficeDev#38 from akrantz/script-cleanup Script cleanup commit 1a39181 Author: Adam Krantz <[email protected]> Date: Fri Jun 7 13:46:02 2019 -0700 update VSCode task to use 'build:dev' commit f16da4e Author: Adam Krantz <[email protected]> Date: Fri Jun 7 13:45:29 2019 -0700 don't need special scripts for testing commit cb1ebef Merge: c175163 658e4c1 Author: Adam Krantz <[email protected]> Date: Fri Jun 7 12:34:20 2019 -0700 Merge pull request OfficeDev#37 from akrantz/build-without-https 'npm run build:dev' instead of 'build-dev' commit 658e4c1 Merge: ec1f66f c175163 Author: Adam Krantz <[email protected]> Date: Fri Jun 7 11:48:16 2019 -0700 Merge branch 'master' into build-without-https commit ec1f66f Author: Adam Krantz <[email protected]> Date: Fri Jun 7 10:26:42 2019 -0700 'npm run build:dev' instead of 'build-dev' commit c175163 Author: Courtney Owen <[email protected]> Date: Fri Jun 7 08:38:28 2019 -0700 Add the ability to run tests (OfficeDev#34) `npm run test` will now run tests using Mocha and Office.js to run the add-in and validate results. commit 70c9f65 Author: Adam Krantz <[email protected]> Date: Fri Jun 7 08:32:20 2019 -0700 enhance webpack config for https and port (OfficeDev#36) * if '--https false' is specified when calling webpack or webpack-dev-server, it will use http instead of https. This will run the dev server using http: npm run dev-server -- --https false This will build without prompting for dev certs: npm run build -- --https false * Get the port from the package.json config "dev-server-port". If the value of this config is changed in package.json, "npm run dev-server" will use it. commit f0a3672 Author: Courtney Owen <[email protected]> Date: Thu Jun 6 14:47:10 2019 -0700 Remove unneeded file commit 4809bb7 Merge: 7722e91 088537a Author: Courtney Owen <[email protected]> Date: Mon Jun 3 15:39:50 2019 -0700 Merge remote-tracking branch 'origin/master' into ui-tests commit 7722e91 Author: Courtney Owen <[email protected]> Date: Mon Jun 3 15:32:54 2019 -0700 Roll back version of clean-webpack-plugin commit 0d77f52 Author: Courtney Owen <[email protected]> Date: Mon Jun 3 14:44:33 2019 -0700 Rename test commit 6b67af8 Merge: 89dc988 bd1bb9d Author: Courtney Owen <[email protected]> Date: Mon Jun 3 14:40:40 2019 -0700 Merge remote-tracking branch 'origin/master' into ui-tests commit 89dc988 Author: Courtney Owen <[email protected]> Date: Fri May 24 10:49:52 2019 -0700 Enable test on Mac and deregister addin during test teardown - Deregistering the addin will make the test more resilient and allow it to run on Mac - Rename test files - Add test-helpers methods commit af0c233 Author: Courtney Owen <[email protected]> Date: Wed May 22 15:07:29 2019 -0700 Only run tests on Windows due to Bug 3377441 - Bug 3377441: Office Addins fail to work on Mac if addin has previously been cached in the WEF folder commit 0c6916b Author: Courtney Owen <[email protected]> Date: Wed May 22 13:44:06 2019 -0700 Remove unnecessary platform check for closeWorkbook - the closeworkbook API call should work for both Mac and Windows commit 0e5a47b Author: Courtney Owen <[email protected]> Date: Wed May 22 09:52:07 2019 -0700 Clean up test code commit cfb4896 Author: Courtney Owen <[email protected]> Date: Tue May 21 14:11:01 2019 -0700 Add ts-node package commit 1810343 Author: Courtney Owen <[email protected]> Date: Tue May 21 13:57:30 2019 -0700 Isolate test code from product code
* CustomFunctions.associate() calls added automatically (OfficeDev#152) * update packages * update import for [email protected] * update custom-functions-metadata-plugin (OfficeDev#155) * enhance webpack config for https and port (OfficeDev#36) * if '--https false' is specified when calling webpack or webpack-dev-server, it will use http instead of https. This will run the dev server using http: npm run dev-server -- --https false This will build without prompting for dev certs: npm run build -- --https false * Get the port from the package.json config "dev-server-port". If the value of this config is changed in package.json, "npm run dev-server" will use it. * Add the ability to run tests (OfficeDev#34) `npm run test` will now run tests using Mocha and Office.js to run the add-in and validate results. * 'npm run build:dev' instead of 'build-dev' * don't need special scripts for testing * update VSCode task to use 'build:dev' * Minor fix to address issue when converting to single host - Without this change, tests will fail for Excel after calling convertToSingleHost with "excel convert-test" params. * add UI tests (OfficeDev#162) * Merge Office-Addin-Taskpane/master * Update CleanWebpackPlugin in webpack.config.js * Roll back version of clean-webpack-plugin to address build error * Update issue templates * Update issue templates * delete un-used template * Bug 3426728: Use correct copyright header in templates (OfficeDev#41) - Fix copyright headers in project html files - Note, I checked the headers in the other project files (.ts and .css) and they are currently fine * Add convertToSingleHost.js to allow for removing test infra * Remove convertToSingleHost script at and of conversion process * bug fixes in convertToSingleHost.js * Delete .github folder during convert-to-single-host * Remove "Preview" from Readme * Merge-office-addin-taskpane/master * Revert version of clean-webpack-plugin to address build failure * Updated sample YAML description * Updated sample YAML description * Merge Office-Addin-Taskpane/master * Merge Office addin taskpane (OfficeDev#179) * update dependencies * update mocha and file-loader * add CODEOWNERS file * convert-to-single-host: delete .github folder * Update packages to latest versions (OfficeDev#47) - Also make small change to launch.json to fix issue that was causing 'Debug Tests' config to fail after updating mocha version. * Update office-addin-scripts packages (OfficeDev#48) -Verified tests pass locally * Remove office-toolbox package (OfficeDev#51) The office-addin-debugging package is being updated to have sideload integrated rather than require using office-toolbox. This removes the sideload and unload package.json scripts which are no longer needed. They could be replaced using a command like: "npx office-addin-dev-settings sideload manifest.xml -a excel" however this would require a dev dependency on office-addin-dev-settings. I think it would be simpler to just use the "start" and "stop" scripts instead. These scripts were required previously so that "office-addin-debugging start" would know how to sideload, but now that this functionality is part of the office-addin-dev-settings package, it can import and call it directly. * Update all package versions (OfficeDev#54) - This will ensure we pick up important office-addin-scripts changes as well * Remove ugly table in readme
…ns.js to customfunctions.ts)