Enable no implicit any in the utilities package #1970
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
christiango
commented
Jun 10, 2017
| for (id in this._immediateIds) { | ||
| if (this._immediateIds.hasOwnProperty(id)) { | ||
| this.clearImmediate(id); | ||
| this.clearImmediate(parseInt(id, 10)); |
Member
Author
There was a problem hiding this comment.
Seems like there is a bug here, but I don't have enough context. Id is a string since it's a key in _timeoutIds, but it's being used as a number...
Member
There was a problem hiding this comment.
I don't think that's a bug, unless it's browser specific. I just tried it by caching the stringified value of the response of setTimeout and then calling clearTimeout with it and it worked. Are you seeing an actual bug?
Member
Author
There was a problem hiding this comment.
I am not seeing an actual bug. It just stood out to me.
christiango
commented
Jun 10, 2017
| @@ -1,10 +1,10 @@ | |||
| import { memoize, memoizeFunction, setMemoizeWeakMap } from './memoize'; | |||
| import * as weakMapPolyfill from 'es6-weak-map/polyfill'; | |||
| import weakMapPolyfill = require('es6-weak-map'); | |||
Member
Author
There was a problem hiding this comment.
Member
There was a problem hiding this comment.
I made a fix to be ok with WeakMap missing in the memoize functions. i think this is ok as long as tests pass.
dzearing
approved these changes
Jun 10, 2017
Member
dzearing
left a comment
dzearing
left a comment
There was a problem hiding this comment.
Awesome job! kudos! Thank you for doing this!
Member
Author
|
Build is broken because of shrinkwrap. I'll investigate next week. It repros without my changes and just running rush generate. |
jspurlin
pushed a commit
to jspurlin/office-ui-fabric-react
that referenced
this pull request
Jun 12, 2017
* Enable no implicit any in the utilities package (microsoft#1970) * Fix no implicit anys in utilities package. * Rush change * Reverse all shrinkwrap changes except the typings one. * Fix bundle minification to exclude debug warnings. (microsoft#1973) * Updating shrinkwrap, rush, and making minify build have production flag to remove debug code. * Updates. * removing lockfile. * dropping to 7. Moving back to npm run build. * Downgrading rush. * Applying package updates. * Website: Update dev.office.com header (microsoft#1966) * Use Fabric Core 7.1.0 for the website * Adjust position of caret's in header so that they spin around central axis * Update to latest icon font from dev.office.com for header and move outside out :global{} to fix build issue * Remove a u- prefix that was missed earlier * Update dev.office.com header with the latest navigation links * Add change file * Make sure the quote rule is enabled for tsline (microsoft#1961) * With responsive mode error (microsoft#1956) * withResponsiveMode: Adding error handling around the case where window.innerWidth throws an exception * adding change log file * Create withResponsiveMode.tsx
christiango
pushed a commit
that referenced
this pull request
Jun 19, 2017
* Add ComboBox functionality * Make a fix for IE where non-allowFreeform is showing the keypresses... * Update the PR with code review feedback. Simplified the code a lot, decoupled the shared props/memebers from comboBox and Dropdown, and extended BaseAutoFill which allowed DynamicAutoFill to be removed * Fixing some build warnings * Update my example page to explicitly not use two mutually exclusive props * The npm-shrinkwrap.json file automatically updated... commiting * Had a bad copy paste, fixing up the incorrectly logic * Removing the fontFamily aspect of the comboBox and utilizing the onRenderOption instead. The fontFamily aspect was too specific for the generic comboBox * Update based on feedback * Jspurlin jspurlin/combo box (#2) * Enable no implicit any in the utilities package (#1970) * Fix no implicit anys in utilities package. * Rush change * Reverse all shrinkwrap changes except the typings one. * Fix bundle minification to exclude debug warnings. (#1973) * Updating shrinkwrap, rush, and making minify build have production flag to remove debug code. * Updates. * removing lockfile. * dropping to 7. Moving back to npm run build. * Downgrading rush. * Applying package updates. * Website: Update dev.office.com header (#1966) * Use Fabric Core 7.1.0 for the website * Adjust position of caret's in header so that they spin around central axis * Update to latest icon font from dev.office.com for header and move outside out :global{} to fix build issue * Remove a u- prefix that was missed earlier * Update dev.office.com header with the latest navigation links * Add change file * Make sure the quote rule is enabled for tsline (#1961) * With responsive mode error (#1956) * withResponsiveMode: Adding error handling around the case where window.innerWidth throws an exception * adding change log file * Create withResponsiveMode.tsx * New Component: Stepper (#1759) * First step at stepper implementation. * Add first implementation of stepper. * Add functionality to stepper * Refine the Stepper class and add tests * let's make sure to put focus back on the text field when submitting via enter * Added documentation to Stepper. * Add flexibility to current stepper implementation. * Modified example implementations. * Add aria-valuemax. * Change Stepper to SpinButton. * Add example with unit. * Implement color scheme in the ContextualMenu control to enable alternative theming. * Improvements to SpinButton. * Fix increment function calls. * Add new width optional parameter. * Add label direction. * Fix border. * Add Position enum. * `defaultValue` is now the deciding prop for using the default implementation or not. * onBlur is now onValidate. * Fix tests. * Fix warnings. * Add implementation for labelGap. * Put some polish on the styling, added some icon support, and added some more example spinButtons * Implement the bar and unit tests and component page * Add the ability for the spinButton buttons to look pressed when spinning via keyboard * Revert "Implement color scheme in the ContextualMenu control to enable alternative theming." This reverts commit 4f830cd. * Don't render an empty icon for an icon-less header menu item. * Revert "Implement Document Title Bar" * update some CSS for high contrast in ff and use css utility instead of concatenating string classnames * Fix quotation issue * Fix Spin Button properties table. * Fix Spin Button example code * Use iconProps instead of string * Extracted `spinning` out of state * Add autobind instead of manually binding private functions to this * Change `+` syntax for more explicit `Number()` * Remove unnecessary cast * Fix typos * `incrementButtonIcon` and `decrementButtonIcon` are now IIconProps * Add KeyboardSpinDirection enum * Fix test description * Fix SpinButton tests * Remove unused onChange callback from SpinButton * Revert onChange * Remove old Stepper.ts file * Use module css instead of global * Fix missing word in comment * Callback functions now allow for void return (state to be updated outside) * Use `_async` instead of window * Fix minor rendering issue with browser zoom * Rename `_spinning` to `_spinningByMouse` for clarity * Fix tests * Fix extra space before label * Remove width outside of SpinButton component and fix styling * Add more tests to SpinButton * Fix SpinButton documentation * Fix typo * Fix AppDefinition for SpinButton and Spinner * Add missing documentation to SpinButton title prop * Various SpinButton fixes * Fix SpinButton path for properties * Fix SpinButton styling issues * Remove labelGap property from SpinButton * merge some changes * merge changes * merge * Fix a new tslint warning after npm installing * Fixing some casing warnings npm start was angry about * Removing an extra line that got added with the last push * Create SelectableOption.ts * Create SelectableOption.ts * Create ComboBox.Props.ts * Create ComboBox.test.tsx * Create ComboBox.Basic.Example.tsx * Create Dropdown.Props.ts * Fix the case sensitivity issue * one more casing issue * changing the reference of utilities in the test file * Actually it look like it has to be pascalCase here * ... really... what's going on with the casing here * Address feedback from in person review with David * A few minor updated to remove uneeded comment and unneeded try/finally in tests * Fix up a typo and fix up to use consistent syntax on a few lines
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Slowly getting us on the road to enabling strict mode for Typescript. I'll be doing these package by package and a single option at a time to help catch any issues. This enables the no implicit any option in the utilities package. Most of the fixes here are not adding new typings, but rather explicitly stating that something is of any type.