Skip to content
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

Simple stub fails for ES6 import #1711

Closed
ctaylo21 opened this issue Feb 27, 2018 · 18 comments
Closed

Simple stub fails for ES6 import #1711

ctaylo21 opened this issue Feb 27, 2018 · 18 comments

Comments

@ctaylo21
Copy link

ctaylo21 commented Feb 27, 2018

  • Sinon version : 4.4.2
  • Environment : MacOSX High Sierra 10.13.3
  • Example URL :
  • Other libraries you are using: Webpack, babel, mocha

What did you expect to happen?
Given a module like so:

// mod1.js
function test() {
  return 'fail';
}

export {
  test
};

And a test like this:

// mod1Spec.js
import { assert } from 'chai';
import sinon from 'sinon';

import * as mod1 from 'Modules/settings/mod1';

describe('test', function () {
  it('should correctly mock module method', () => {
    sinon.stub(mod1, 'test').returns('pass');
    assert.strictEqual(mod1.test(), 'pass');
  });
});

The test should pass.

What actually happens
The method is never mocked. It still returns 'fail' even with the direct stub above it. I've seen some issues around mocking ES6 modules, but they all seem to imply that using import * as blah will allow you to stub correctly (ES6 classes may be a different story).

Can you actually stub ES6 modules with sinon? If so, are there any known libraries that can interfere with the stubbing? I imagine babel or webpack would be valid culprits as they may bundle/transpile modules and break stubbing.

I've been tearing my code apart, so I want to confirm that it's possible to stub ES6 modules without using anything like babel-plugin-rewire or if something like that plugin is now required.

How to reproduce
See code above.

@mroderick
Copy link
Member

I've created a runnable test from your example, using @std/esm instead of babel.

It shows very clearly that (under node), we're not allowed to modify modules that are imported using import. It looks like imports are read-only views on exports.

So, sinon.stub is not able to modify the imported module.

I guess you will have to use some link seam with your chosen module loader to redefine what the dependencies mean.

If you're going to use a link seam, you might be interested in the new sinon.fake api (install as sinon@next). The api is still under development, feedback is very welcome.

I wonder if there's a way for us to detect that we're using modules, and thus can't replace properties on imports. If there is, at least we can provide a useful error message to users, so they won't have to spend a lot of time figuring out why things are failing in unexpected ways.

@fatso83
Copy link
Contributor

fatso83 commented Feb 28, 2018

This is outside the scope of what sinon is supposed to do, as it deals with a whole range of complexity that is outside of our control. If you open this codepen in Chrome and open the console, you will see it is not possible to alter a module's exports in a fully spec compliant system (which Babel is not, but Chrome is), resulting in

Cannot assign to read only property 'test' of object '[object Module]'

Babel is another matter entirely, as the code produced when transpiling ES6 modules to ES5 CommonJS modules doesn't conform to the ES6 module spec (parse time vs dynamic evaluation time exports, etc), and is more or less an internal detail of Babel. That means the code above should work, but I messed enough around with just setting up that simple Codepen to demonstrate ES6 loading that I won't go down that additional rabbit hole ...

See the tips mentioned in #1623 for going further (proxyquire or rewire are probably required). Closing as out of scope.

@fatso83 fatso83 closed this as completed Feb 28, 2018
@fatso83
Copy link
Contributor

fatso83 commented Feb 28, 2018

@mroderick I used 2 hours in from starting to answer this until actually submitting the reply, so didn't see your reply until after I submitted. If this is really a feature request for better error messages I think we should open a separate issue to make the reading a bit clearer.

@mroderick
Copy link
Member

If this is really a feature request for better error messages I think we should open a separate issue to make the reading a bit clearer.

I was looking into whether or not it is even possible to detect when running in ESM compliant mode. AFAICT, it is not.

I suppose we can make an improvement by wrapping the replacement of the property in a try-catch, and then give a nicer error message. Shall we do that?

@fatso83
Copy link
Contributor

fatso83 commented Feb 28, 2018

For cases like above, where we transpile, detecting probably won't work, but for ESM shouldn't we be able to just do a type check on objects using toString and === '[object Module]'?

@mroderick
Copy link
Member

shouldn't we be able to just do a type check on objects using toString and === '[object Module]'?

Good idea

In my experiments (in node 8.9.4), a module import doesn't have a toString method, and using Object.prototype.toString returns [object Object].

Maybe I am missing something. Do you you have a working example of this?

@fatso83
Copy link
Contributor

fatso83 commented Feb 28, 2018

I don't - I just assumed the toString() method is where the error (mentioned above) got the [object Module] bit from. I can try looking into it a little bit.

@fatso83
Copy link
Contributor

fatso83 commented Feb 28, 2018

OK, looked into it, and I think you have missed something 😏 The behaviour I saw in Chrome was reproduced in Node 6.12, 8.0, 8.7 and 9.2. I found the required info in the Ecma 262 spec for Module and this snippet on MDN.

I basically forked your repo and added some tests to verify it:

  the Module object
    ✓ should have a toStringTag symbol with the expected value
    ✓ should use the toStringTag symbol 

This should be a trivial change, so maybe I can just supply a PR to throw an error for it?

@ctaylo21
Copy link
Author

@fatso83 @mroderick Thanks a ton for the replies. I totally understand that this is outside scope of sinon so I appreciate the help in understanding the issue.

I really like the idea of a warning/error when users try to stub modules imported using import. Hopefully that will help anyone attempting to do the same thing I did.

I'll have to explore alternatives or potentially just stick with RequireJS and not "upgrade" to ES6 import/export.

fatso83 added a commit that referenced this issue Feb 28, 2018
Ref #1711 and #1623 for background.

This feature is simply about throwing a meaningful error, instead
one that tells the user that the property cannot be deleted. As
the exports from ECMAScript modules are immutable once created we
cannot do anything about them from the view of Sinon, so it's
preferable to simply tell the user that when we can.

This should not affect transpiled code (say, using Babel), as the
resulting modules are no longer true ECMAScript modules (but rather
some form of CommonJS module).
@fatso83
Copy link
Contributor

fatso83 commented Feb 28, 2018

Pull request in queue: #1715.

Check out the diff, @ctaylo21, to see if this is approximately what you would expect if running in a ES Module support system.

I don't think this would necessarily help you out in your specific case, unfortunately, as a transpiled ES Module is no longer a true ES Module ...

fatso83 added a commit to fatso83/sinon-1711-babel that referenced this issue Feb 28, 2018
Ref sinonjs/sinon#1711

It was said that the test failed. It did not, as suspected.
@fatso83
Copy link
Contributor

fatso83 commented Feb 28, 2018

@ctaylo21 As I mentioned above, I was surprised to hear that the code you supplied did not work after transpiling. I just tried recreating the issue with a fresh project that uses Babel to transpile your example and it works fine, so I suspect you are not doing exactly what that example code shows?

I suspect you have fallen victim to the same issues as #1248 and #1648 that basically has to do with how one in a test can stub exports in a module that is used by another module. This is basically a CJS issue, so read up on that using the links below, as you can achieve it without proxyquire and similar "require middleware" (working with link seams). I think you'll find these links interesting for achieving what you want if you want to avoid extra dependencies:

@ctaylo21
Copy link
Author

ctaylo21 commented Mar 1, 2018

@fatso83 I can confirm that your branch does not throw an exception when I try to stub an imported module in my code. I verified the check

if (
       object &&
       typeof Symbol !== "undefined" &&
       object[Symbol.toStringTag] === "Module"
   ) {
       throw new TypeError("No support for stubbing ES Modules");
   }

is false and I assume this is likely due to Babel transpiling the code.

I don't know why your clean setup works but mine doesn't. I'll have to dig into that.

My main issue is that I'm trying to upgrade a large codebase off a legacy build system. As a part of that refactor, I was hoping to use import/export and stop using RequireJS. Dependency injection and/or middleware are valid solutions, but would likely require a lot of rewriting of thousands of tests. It seems like our options are either rewrite a lot of our modules, use link seams, or stay with RequireJS.

Thanks again for all of your help, I really appreciate it.

@mroderick
Copy link
Member

If you do decide to go with link seams targeting module loaders, here are my favourites

I know that there are similar solutions for when using Webpack, but I don't have enough recent experience with Webpack to recommend any solution for that.

If I was faced with your challenge, I would do a little experiment to see if I could get things and running with System.js, as it can consume all three module systems. This would allow you to refactor the module parts of the code slowly, instead of porting the entire codebase in one go, and risking de-stabilising the whole system.

Another tool that might be useful in your arsenal: codemod can help you rewrite all the AMD code to CommonJS/ESM, including the code for the tests.

@ctaylo21
Copy link
Author

ctaylo21 commented Mar 1, 2018

@fatso83 It appears that my basic test was failing because of the modules setting in babel. I had it set to false which told Babel not to convert the module format. The default transforms modules into CommonJS format, which appears to be why your sample repo with babel worked.

Since tree-shaking with Webpack doesn't work on modules without a static structure, the recommended setting is to disable module transforming in Babel (as long as you are using ESM syntax). So most people using webpack and babel will likely see the same issue as I did.

fatso83 added a commit that referenced this issue Mar 5, 2018
Ref #1711 and #1623 for background.

This feature is simply about throwing a meaningful error, instead
one that tells the user that the property cannot be deleted. As
the exports from ECMAScript modules are immutable once created we
cannot do anything about them from the view of Sinon, so it's
preferable to simply tell the user that when we can.

This should not affect transpiled code (say, using Babel), as the
resulting modules are no longer true ECMAScript modules (but rather
some form of CommonJS module).
@fatso83
Copy link
Contributor

fatso83 commented Mar 5, 2018

@ctaylo21 Maybe you are running Babel 5, because that setting is no longer supported. Enabling it in my repo gives this:
ReferenceError: [BABEL] src/mod1.js: Using removed Babel 5 option: foreign.modules - Use the corresponding module transform plugin in the plugins option. Check out http://babeljs.io/docs/plugins/#modules

These days it seems one has to explicitly enable a certain transform, but I am not sure how to dig into this further ...

@ctaylo21
Copy link
Author

ctaylo21 commented Mar 8, 2018

Hm, I thought I had the latest packages with Babel. Might be a combination of packages or something. Either way, it's fine to drop this. I understand why my issue happened and why babel can possibly "fix it" because it can transform modules to different formats. I sincerely appreciate all the help and advice.

giladgray added a commit to palantir/blueprint that referenced this issue Jan 9, 2019
* upgrade to webpack 4

* migrate webpack config

* add cssnano, though it doesn't seem to work

* fix cssnano in docs-app

* small config refactors

* upgrade karma deps

* migrate karma webpack usage.

use webpack alias for enzyme adapter instead of template string

* upgrade sinon

* skip borked table tests that rely on now-impossible sinon spy behavior.

sinonjs/sinon#1711

* fix lint

* Use cjs modules for karma testing

* Forgot to enable cjs for table
NachoJusticia pushed a commit to graphext/blueprint that referenced this issue Apr 12, 2019
* [DateInput] Add support for datepicker clearButtonText and todayButtonText props. (palantir#3020)

* [DateInput] Add datepicker clearButtonText and todayButtonText props support.

* [DateInput] Add unit test for clearButtonText and todayButtonText props.

* include integrity hashes in yarn lock (palantir#3035)

* Publish

 - @blueprintjs/docs-theme@3.0.2

* Publish

 - @blueprintjs/docs-theme@3.0.3
 - @blueprintjs/karma-build-scripts@0.9.0
 - @blueprintjs/node-build-scripts@0.8.0
 - @blueprintjs/webpack-build-scripts@0.7.0

* Fix reset of active item when query hasn't changed (palantir#3072)

* Fix reset active item when query hasn't changed

* Address comments

* [DateInput] close on tab key press (palantir#3038)

* Publish

 - @blueprintjs/datetime@3.3.1
 - @blueprintjs/select@3.2.1

* Correct Table reference to HTMLTable (palantir#3075)

Also correct link to html-table page.

* 🔧 fix dependencies (palantir#3078)

* node-gyp resolution

* * dep for build-scripts packages

* yarn.lock

* preview script won't fail build without GH_AUTH_TOKEN

* update yarn min version in readme

* fix icons alignment (palantir#3102)

* [Breadcrumb] Add current prop (palantir#3104)

* Render breadcrumb children (palantir#3105)

* fix checkbox indicator display (palantir#3113)

* Only set default font-family on body selector (palantir#3115)

* [FormGroup] add contentClassName and style props (palantir#3120)

* add contentClassName prop

* add style prop

* [Portal] add container prop (palantir#3045)

* [Portal] add container prop

* [Overlay] add container prop

* [Popover] add container prop

* [Tooltip] add container prop

* [Dialog] add container prop

* [Alert] add container prop

* fix test failed

* [portal] avoid possible crash on componentDidMount

* rename container to portalContainer for overlay components

* chore: remove useless code

* add isotest for className prop (palantir#3119)

* four args become two. add className isotest.

refactor generateIsomorphicTests() to use config object in mapped type

* refactor isotest suites to new config object

* [Tag] fix line-height for centering (palantir#3117)

* fix lint error

* set tag line-height to icon size

* $tag-line-height(-large) vars

* fix label documentation example (palantir#3087)

* fix label documentation example

* Use class constants

* remove first id

* Label B

* added new icon (palantir#3052)

* [table] Perf improvements with selections (palantir#2005)

* table performance improvements - reducing rerendering where possible

* reinstate enum strings

* fix compile errors

* fix lint errors

* fix test

* cell: revert changes

* code review suggestions; add forceRerenderOnSelectionChange flag

* lint

* default to false

* remove unused logo styles from navbar (palantir#3123)

* [Breadcrumbs] Add new component (palantir#3106)

* Add breadcrumbs component

* Use lambda syntax

* Add overflow list props

* Disable overflow menu items

* Add missing prop docs

* Document new Breadcrumb special case

* Add Breadcrumbs prop docs

* Use ul tag

* Add test to OverflowList

* Add tests to Breadcrumb

* Add tests for Breadcrumbs

* Fix docstring

Co-Authored-By: invliD <mail@invlid.com>

* Improve docstring

Co-Authored-By: invliD <mail@invlid.com>

* Fix Breadcrumb docs

* Fix linebreak

* Default breadcrumbRenderer

* Explain why the menu is reversed

* Add link to OL

* Add code example

* Re-use breadcrumbs example using different name

* docs edits

* Publish

 - @blueprintjs/core@3.8.0
 - @blueprintjs/icons@3.3.0
 - @blueprintjs/table@3.3.0

* fix disappearing caret on focus of HTMLSelect in ControlGroup (fixes palantir#3003) (palantir#3150)

* Single Month Only Prop in DateRangePicker (palantir#3142)

* allow singleMonthlyOnly in DateRangePicker

* updated docs comment

* added singleMonthOnly props in DateRangeInput

* added DateRangeInput test to ensure prop is passed

* updated documentation and example labels

* Allow users to browse omnibar options without query (palantir#3130)

* Using tagName JSX variable for MenuItem (palantir#3061)

* Using tagName JSX variable for MenuItem

* Fixed minnor default issues as per the requests on PR

* [Overlay] add portalClassName to IOverlayableProps (palantir#3148)

* add portalClassName to IOverlayableProps

* fix overlay test

* unmount dialog test

* fix HTML_TABLE_CONDENSED name (reverts 3.x break) (palantir#3147)

* Don't clear TagInput inputValue state if controlled (palantir#3137)

* Don't clear TagInput inputValue state if controlled

Fixes palantir#3134

* Address PR comments

* Revert fixes

* remove margin on heading-only callouts (palantir#3157)

* PopoverPosition = Position + auto values (palantir#3156)

* [Select] Flag to optionally scroll to active item for select components (palantir#3096)

* Flag to optionally scroll to active item for select components

* Respect click and keyboard interactions

* Change docs

* edit prop docs

* add uncontrolled note

* docs edits (palantir#3161)

* [tslint] blueprint-html-components fixer! (palantir#3162)

* enable strict mode in tslint-config

* sort imports before adding

* blueprint-html-components fixes imports

* refactors

* replaceTagName util

* test formatting

* add test for all imports

* h2/h5

* ts-lint --fix (palantir#3159)

* relaxed typings for optional JSX.Elements (palantir#3118)

* export type OptionalElement = JSX.Element | boolean | null | undefined;

use it in `icon` props

* more uses of OptionalElement

* Icons docs uses props

* MaybeElement, false

* [Popover] add boundary prop for easier modifiers (palantir#3149)

* Popover add boundariesElement prop for easier modifiers

* rename to boundary

* rename type to PopperBoundary

* fix examples

* Publish

 - @blueprintjs/core@3.9.0
 - @blueprintjs/datetime@3.4.0
 - @blueprintjs/select@3.3.0
 - @blueprintjs/tslint-config@1.7.0

* Fixed bug where new timeout prop passed to toastr update was ignored (palantir#3164)

* Fix change activeItem when query changes on props for QueryList (palantir#3180)

* Fix change activeItem when query changes on props for QueryList

* move willRecieveProps to componentDidUpdate

* put back

* fix import. VSCode - I ahte you

* Replace calendar months const enum with enum (palantir#3182)

* Include "enable preview comments" in PR templates (palantir#3201)

* [select] fix inputprops docs (palantir#3167)

* [HTMLTable] fix html props type (palantir#3170)

* Add elevation variables in docs (palantir#3202)

* FIXED: @HotkeyTarget -> @HotkeysTarget typo (palantir#3208)

* [Popover/DateInput] fix tabIndex & inputRef support (palantir#3200)

* fix Popover openOnTargetFocus tabIndex logic

* refactor DateInput inputRef logic & add tests

* fix import path

* add corresponding InputGroup tests

* update sandbox URL (palantir#3206)

* [NumericInput] ❤️  (palantir#3204)

* ref type | null

* remove <T> on react events

optional type!!

* Keys.isKeyboardClick

it's like a method on a java enum!

* total render refactor

- FIX: css class is applied when buttons=none
- renderButtons() and renderInput()
- conditionals to place buttons before or after input
- remove special-casing for buttons=none

* generate button event handlers, fix key holding

one little function to cache handler objects.
fix holding enter/space to continuously change value.

* onKeyUp handler is obsolete

Button handles enter/space click logic.
onKeyDown is actually enough - quite elegant.

* remove isButtonGroupFocused state

set but never read

* move shouldSelectAfterUpdate to state so it triggers updates

for the selection logic in cmpDidUpdate

* required state, missed a <T>

* update and refactor tests

not much needed here

* pull pure methods out to utils file

* fix tabs usage

* little things: public state, {value}

* orgainze

* missed a reset

* fix return type of Icon.render() (palantir#3190)

* [DateRangePicker] Remove unused styling in daterangepicker.scss (palantir#3199)

* Remove unused styling in daterangepicker.scss

* Configure CircleCI previews

* [Tree] add node mouseenter and mouseleave events (palantir#3211)

* [Popover] add targetProps (palantir#3213)

* add safeInvokeMember util

* add Popover targetProps, using new util for overridden events

* tests

* only one spread

* add note about ref

* add notes about types

* Circle bot logs artifact URLs if auth token is missing (palantir#3209)

* upgrade circle-github-bot

* refactor script so it logs artifact URLs if auth token is missing

* Add htmlTitle prop to Icon (palantir#3216)

* add htmlTitle prop to Icon

* try to kick circleci?

* comment update & backticks

* add "on hover"

* Added new icons (palantir#3222)

* update contributing notes: DO NOT enable Circle for forks. (palantir#3217)

* update contributing notes: DO NOT enable Circle for forks.

* finish the sentence

* [Popover/Tooltip] add public reposition() method to schedule popper update (palantir#3215)

* [Popover] public reposition method to schedule popper update

* add tooltip.reposition()

* [DateRangePicker] Fix caption behavior in contiguous month mode (palantir#3198)

* Add failing tests

* Use two DayPickers for contiguous months

This is so that we can customize the behavior for captions, so the left
caption changes the start month/year and the right caption changes the
end.

* Add additional rendering tests

* Lint

* Configure CircleCI previews

* Add enforcement for contiguousMonth toggling

* Publish

 - @blueprintjs/core@3.10.0
 - @blueprintjs/datetime@3.5.0
 - @blueprintjs/icons@3.4.0
 - @blueprintjs/select@3.4.0

* Update hotkeys.md (palantir#3227)

* Added EditableText input type field (palantir#3235)

* Fix setState calls using updater functions (palantir#3247)

React does not guarantee that the state changes are applied immediately. It is best practice to use an updater function if the new state relies on the current state.

* Add menu item classnames (palantir#3251)

* Add menu item label classname

* Add text classname prop

* Alphabetize props & improve docs

* docs: tag-input: fix garbled unescaped html appearing due to hard line breaking in md source (palantir#3266)

* Fix dialog css docs (palantir#3271)

* Added esnext as a build target. (palantir#3230)

* [docs] Suggest using `ContextMenuTarget` as a HOC (palantir#3259)

Instead of falling back to the imperative API, suggest using `ContextMenuTarget`, where decorators are not an option.

* [docs] show optional isDarkTheme parameter in ContextMenu API docs (palantir#3273)

To make it more obvious that you can manually create a dark context menu, as raised in palantir#3229

* [ResizeSensor] try/catch findDOMNode to handle possible error state (palantir#3236)

*  🔧 upgrade to webpack 4 (palantir#3252)

* upgrade to webpack 4

* migrate webpack config

* add cssnano, though it doesn't seem to work

* fix cssnano in docs-app

* small config refactors

* upgrade karma deps

* migrate karma webpack usage.

use webpack alias for enzyme adapter instead of template string

* upgrade sinon

* skip borked table tests that rely on now-impossible sinon spy behavior.

sinonjs/sinon#1711

* fix lint

* Use cjs modules for karma testing

* Forgot to enable cjs for table

* Popover docs: inline -> usePortal={false} (palantir#3277)

I _think_ inline is outdated b/c I don't see docs on that prop above. I'm pretty sure this section means to reference `usePortal={false}` instead.

* [docs] fix CDN usage & bundle externals (palantir#3276)

* update webpack externals

* update CDN guide in getting started

* push CDN section to the bottom (to avoid distraction)

* remove repeated install notes

* remove jquery

* Publish

 - @blueprintjs/core@3.11.0
 - @blueprintjs/datetime@3.6.0
 - @blueprintjs/icons@3.5.0
 - @blueprintjs/select@3.5.0
 - @blueprintjs/table@3.4.0
 - @blueprintjs/timezone@3.2.0

* refactor toast timeout update logic to be more obvious (palantir#3275)

always clear timeout when starting to cancel any previous now-outdated active timer.

* [OverflowList] Always re-render items (palantir#3278)

* Allow overriding of heading typography in Sass (palantir#3283)

* [TimezonePicker] Support a custom target via <children> (palantir#3287)

* Export getTimezoneMetadata(), add default value for date parameter

* Support one custom child in <TimezonePicker>

* Update example

* Update tests

* (nit) Rename to CustomTimezonePickerTarget

* Add more docs about possibly ignored props

* Update docs

* Respond to @giladgray CR

* Create CHANGELOG.md

* Suggest query remains when closed if nothing is selected. (palantir#3289)

* Allow shortcuts to change time in Date Range Picker (palantir#3223)

* Allow date range shortcuts to change time

* Address PR comments

* lint

* [Suggest] add `disabled` and `resetOnClose` props (palantir#3292)

* add Suggest resetOnClose prop a la Select

* add Suggest disabled prop

* add resetOnClose switch to example

* docs note

* [QueryList] Initialize activeItem state correctly based on prop. (palantir#3290)

* Initialize QueryList's activeItem state correctly based on prop.

* Fix comment typo

* Code cleanup

* [PanelStack] Allow any prop type for initial panels (palantir#3295)

* [Tooltip] allow HOVER interactions (palantir#3291)

* [Tag] fill & [TagInput] intent props (palantir#3237)

* add Tag fill prop

* add TagInput intent prop

* add tag fill styles & example switch

* docs: Fix link to Popover (palantir#3300)

* Add directory details to packages/*/package.json (palantir#3301)

Specifying the directory as part of the `repository` field in a `package.json`
allows third party tools to provide better support when working with monorepos.
For example, it allows them to correctly construct a commit diff for a specific
package.

This format was accepted by npm in
[npm/rfcs#19](npm/rfcs#19).

* Add directory to repository details in package.json files (palantir#3299)

Specifying the directory as part of the `repository` field in a `package.json`
allows third party tools to provide better support when working with monorepos.
For example, it allows them to correctly construct a commit diff for a specific
package.

This format was accepted by npm in npm/rfcs#19.

* [select] Add optional "itemsEqual" comparator prop to QueryList and related components (palantir#3285)

* Improve QueryList and related components to support optional
equality test implementation for non-primitive item types.

* Revert change to use undefined for representing no selection

* Finish revert change to use undefined for representing no selection

* Remove TODO

* Remove unnecessary param documentation

* Rename "areValuesEqual" prop to "itemsEqual"

* Improve "itemsEqual" prop to also support a property name.

* Documentation typos

* Documentation improvements

* Update copyright year in new file

* Rename new unit test file

* [DRP] Shortcuts support changing time & docs (palantir#3293)

* shortcut shouldChangeTime => includeTime

* revert unnecessary test changes

* add Shortcuts docs to DRP

* Publish

 - @blueprintjs/core@3.12.0
 - @blueprintjs/datetime@3.7.0
 - @blueprintjs/docs-theme@3.0.4
 - @blueprintjs/icons@3.5.1
 - @blueprintjs/karma-build-scripts@0.9.1
 - @blueprintjs/labs@0.16.3
 - @blueprintjs/node-build-scripts@0.8.1
 - @blueprintjs/select@3.6.0
 - @blueprintjs/table@3.4.1
 - @blueprintjs/test-commons@0.8.1
 - @blueprintjs/timezone@3.3.0
 - @blueprintjs/tslint-config@1.7.1
 - @blueprintjs/webpack-build-scripts@0.7.1

* [select] consistency refactors (palantir#3305)

* code consistency across `select` components:

- public state = ... (not inconstructor)
- non-optional refs
- safeInvokeMember to reduce destructuring

* fix tests, explicit null check

* dedupe shortcuts prop types (palantir#3311)

* [NumericInput] allows min === max (palantir#3296)

* NumericInput allows min === max

* add test

* [NumericInput] new rightElement prop (palantir#3314)

* Update select-component.md (palantir#3319)

* Fixing typo (palantir#3318)

Missing "and"

* Added new inbox icons (palantir#3335)

* [docs] click to download sketch files from Resources page (palantir#3334)

* click to download sketch files from Resources page

* lint

* use /raw/ url to download actual file

* rename resource files so they download correctly

* add "Missing fonts?" callout

* minor style polish

* Add internal text to switch component (palantir#3327)

* switch text v0

* font size a bit smaller

* add an example of internal text property

* test

* cleanup

* lint fixes

* review comments

* remove whitespace in tests

* more review comments

* even more review changes

* remove errant property

* restore comment

* update prop docs

* additional tests

* remove unnecessary checks

* [Spinner] SIZE_SMALL = 20 (palantir#3342)

* Spinner.SIZE_SMALL = 20

* fix isotest

* [NumericInput] countDecimalPlaces for number type (palantir#3337)

Fixes palantir#3336

* New Drawer component (palantir#2957)

* add files

* fixed styling

* dark theme added

* Added vertical orientation

* changed viewport units

* Added small and large sizes

* Standardized sizing

* backdrop fixed

* add tests

* small/large => one size prop

* update size constants

* add hasBackdrop switch to example

* use Drawer for API browser

* size prop docs

* fix isotest

* add size+vertical tests, actually run suite

* [DatePicker] fix onChange on month change after initial day selection (palantir#3343)

* fix onChange doesn't fire on month change after initial day selection
Fixes palantir#3340

* change !== to != operator

* add test onChange fired when month is changed for uncontrolled datepicker

* Publish

 - @blueprintjs/core@3.13.0
 - @blueprintjs/datetime@3.7.1
 - @blueprintjs/icons@3.6.0
 - @blueprintjs/select@3.6.1

* Corrected the type of Popover's onInteraction prop (palantir#3348)

* ButtonGroup supports "fill" prop when Buttons are wrapped in Popovers (palantir#3347)

* 🔧 upgrade tslint (palantir#3315)

* Fix shortcuts.tsx react import typings (palantir#3356)

* [Tabs] add panelClassName prop (palantir#3351)

* Fix lint (palantir#3374)

* [Core] Fixed not applying intent color for icon inside tree node (palantir#3355)

* [Core] Add option to make tree node disabled (palantir#3354)

* Drawer default prop warning fix (palantir#3382)

* [Select] Create-able Select components (palantir#3381)

* Upgrade dev dependencies and types (palantir#3370)

* Publish

 - @blueprintjs/core@3.14.0
 - @blueprintjs/docs-app@3.14.0
 - @blueprintjs/select@3.7.0

* [Core] Overlay: fix transitionDuration documentation (palantir#3391)

* [Core] Fix react types regression (palantir#3390)

* Publish

 - @blueprintjs/core@3.14.1
 - @blueprintjs/docs-app@3.14.1

* Publish

 - @blueprintjs/datetime@3.7.2

* [Core] Checkbox: fix indeterminate state update (palantir#3409)

Fixes palantir#3408

* Minor README updates

* [Core] Docs: fix small typo (palantir#3411)

"accomodate" to "accommodate"

* [Docs] Overlay Example: add option to test scrolling behavior (palantir#3406)

* Update Bug_report.md

* [datetime] docs: fix react-day-picker API hyperlink (palantir#3435)

* [core] TagInput: use undefined instead of null to hide clearButton (palantir#3436)

* [Select] hide QueryList item list when menuContent and createItemView are both null (palantir#3426)

* [MultiSelect] Enable pasting of multiple values (same as TagInput) (palantir#3428)

* [Core] Add z-index to panel stack header (palantir#3414)

* [Core] Add option to grow text area with input (palantir#3398)

* [Core] TextArea: fix style object regression (palantir#3440)

* [Core] Add position prop to Drawer component, deprecate vertical prop (palantir#3386)

* [Table] Fix resizable props documentation (palantir#3400)

* Switch to standard Apache-2.0 license (palantir#3457)

* [table] Fix documentation to use the right css file name (palantir#3452)

* Prepare release v3.15.0 (palantir#3458)

* Publish

 - @blueprintjs/core@3.15.0
 - @blueprintjs/datetime@3.8.0
 - @blueprintjs/docs-app@3.15.0
 - @blueprintjs/docs-theme@3.1.0
 - @blueprintjs/icons@3.7.0
 - @blueprintjs/karma-build-scripts@0.10.0
 - @blueprintjs/node-build-scripts@0.9.0
 - @blueprintjs/select@3.8.0
 - @blueprintjs/table@3.5.0
 - @blueprintjs/test-commons@0.9.0
 - @blueprintjs/timezone@3.4.0
 - @blueprintjs/tslint-config@1.8.0
 - @blueprintjs/webpack-build-scripts@0.8.0

* [Core] File input text styling for active selections (palantir#3375)

* Fix root package.json file that had duplicated resolutions

* Add yarn lock from upstream

* Update graphext packages versions
franck-romano pushed a commit to franck-romano/sinon that referenced this issue Oct 1, 2019
Ref sinonjs#1711 and sinonjs#1623 for background.

This feature is simply about throwing a meaningful error, instead
one that tells the user that the property cannot be deleted. As
the exports from ECMAScript modules are immutable once created we
cannot do anything about them from the view of Sinon, so it's
preferable to simply tell the user that when we can.

This should not affect transpiled code (say, using Babel), as the
resulting modules are no longer true ECMAScript modules (but rather
some form of CommonJS module).
@mrdulin
Copy link

mrdulin commented Nov 19, 2019

I am facing the same issue.
a.ts:

export function funcA(args) {
  return function c() {};
}

funB.ts:

import * as a from './a';

export const funcB = () => a.funcA({ arg: 123 });

funcB.test.ts:

import { expect } from 'chai';
import sinon from 'sinon';
import * as a from './a';
import { funcB } from './funcB';

describe('57796168', () => {
  let sandbox: sinon.SinonSandbox;
  before(() => {
    sandbox = sinon.createSandbox();
  });
  it('should call a.funcA', () => {
    const funcAStub = sandbox.stub(a, 'funcA');
    funcB();
    expect(funcAStub.calledWith({ args: 123 })).to.be.true;
  });
});

Unit test result:

  57796168
    1) should call a.funcA


  0 passing (16ms)
  1 failing

  1) 57796168
       should call a.funcA:

      AssertionError: expected false to be true
      + expected - actual

      -false
      +true
      
      at Context.<anonymous> (src/stackoverflow/57796168/funcB.test.ts:14:54)

@fatso83
Copy link
Contributor

fatso83 commented Nov 20, 2019

@mrdulin This is a closed issue, meaning it has been resolved. It is also unclear what you are also seeing, explicitly, as it's not clear which environment you are running the code in (pure Node, Babel transforms, Webpack, ... etc).

If you need some assistance in getting things working, please post it to StackOverflow and tag it with sinon, so the bigger community can help answer your questions.

If you feel that your topic is an actual new issue with Sinon, please open a new ticket and follow the guidelines for reporting an issue.

Before you do that, please read through this thread carefully and try to understand what Sinon actually can do something about, and what is otherwise outside the scope of Sinon (such as intricacies caused by your module bundler, like Webpack or Babel).

@sinonjs sinonjs locked as resolved and limited conversation to collaborators Nov 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants