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

Fail TypeScript import #106

Closed
wants to merge 2 commits into from
Closed

Fail TypeScript import #106

wants to merge 2 commits into from

Conversation

jednano
Copy link
Contributor

@jednano jednano commented Dec 10, 2016

This PR shows that importing classNames (as a default module) into TypeScript does not work and that #105 fixes the issue.

@chrisbuttery
Copy link

@jedmao I came across this issue today. Thanks for looking into it.

@jednano
Copy link
Contributor Author

jednano commented Apr 25, 2018

Strangely, this PR uses an "unknown repository"? Couldn't figure it out, so I created a new PR, #162.

@chrisbuttery I'm not sure how you ran into this since #105 was merged in last year. Perhaps it was never published?

@jednano jednano closed this Apr 25, 2018
@jednano
Copy link
Contributor Author

jednano commented Apr 25, 2018

@chrisbuttery see #105 (comment)

bryphe pushed a commit to onivim/oni that referenced this pull request May 16, 2018
* Add middle click tab closing (#2069)

by adding a onMouseDown directive to both tab file icon and
tab name and checking the resulting React.MouseEvent for a
middle click

* Fix unused imports and whitespace in Tabs ui test (#2069)

* Update Tabs component test snapshot (#2069)

* Fix Tabs ui test to correctly retrieve children (#2069)

* Differentiate between single und multiple tabs in test (#2069)

* Use mousedown event for tab selection and closing (#2069)

by middle clicking and let the tab itself handle the logic
for it by checking React.MouseEvent.button value

* Update classnames dependency to lastest master branch

and write own @types module declaration so that it can be used in
testing nstead of using a mock. The mock does not suffice as the
way the actual module previously had to be imported was
"import * as classNames" where classNames was the actual function.

It is not possible to build a module in typescript/es6 imports, which
will directly return a function in the same way the dependency did
in commonjs module syntax. Instead when defining a function to be
returned as default export it is returned as an Object like this
"{ default: [Function] }", which is correctly resolved when importing
with "import classNames from 'classnames'".

This only previously worked in production as webpacks ts-loader,
handles this issue, whereas when testing the sources are only compiled
with tsc.

There is an update to the classnames dependency on the current master,
but there hasn't been a release since 2006. So options were to setup
webpack for tests as well or add updated classnames dependency which
sets a "default" value on its commonjs exports.

Links for reference:
JedWatson/classnames#152
JedWatson/classnames#106
DefinitelyTyped/DefinitelyTyped#25206
microsoft/TypeScript#2719

* Fix tab click onMouseDown callback

* Test tab clicks to select/close trigger callbacks

* Mock child react components directly to test smaller unit

* Reset calls to callback mocks in each test case

* Add tests for tabs interaction with FileIcon/Sneakable
kittle31 added a commit to kittle31/material-components-web-react that referenced this pull request Feb 28, 2019
a fix for this error:
JedWatson/classnames#106
was added in classnames v 2.2.6.

update dependency version from 2.2.5 to 2.2.6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants