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

build(typescript): Removes typescript and tsgen #295

Merged
merged 1 commit into from Feb 8, 2018
Merged

Conversation

bhough
Copy link
Contributor

@bhough bhough commented Feb 8, 2018

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250
@codecov
Copy link

codecov bot commented Feb 8, 2018

Codecov Report

Merging #295 into version-2 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           version-2   #295   +/-   ##
========================================
  Coverage        100%   100%           
========================================
  Files             70     70           
  Lines            388    388           
  Branches         102    102           
========================================
  Hits             388    388

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee912ff...1ff6a83. Read the comment docs.

@bhough bhough merged commit 77478c2 into version-2 Feb 8, 2018
@bhough bhough deleted the remove-tsgen branch February 8, 2018 01:49
@ForbesLindesay
Copy link
Contributor

What!!!!!!!!!!!!!!!!!!!!!!! I put a fair bit of work into adding tyepscript support for this library, and was happy to help maintain it. Why is it suddenly getting removed without warning?

@mxstbr
Copy link
Member

mxstbr commented Feb 8, 2018

:thinking_face:

@mxstbr
Copy link
Member

mxstbr commented Feb 8, 2018

I'm curious too

@ForbesLindesay
Copy link
Contributor

@mxstbr can we revert this and maybe open issues on tsgen for anything that needs fixing to resolve this?

@mxstbr
Copy link
Member

mxstbr commented Feb 8, 2018

Yeah sure, I wonder why @bhough removed that though. Haven't stayed up to date with dev of this pkg so don't have any bg information.

@bhough
Copy link
Contributor Author

bhough commented Feb 8, 2018

@ForbesLindesay @mxstbr I'm removing it because it has caused a variety of issues that have either had an issue filed directly (ForbesLindesay/tsgen#2) or have pinged you on on our issue (#250). And it appears that it may not even be fully working (#276).

It has blocked a variety of modules, contributions, and changes from going live and I needed to move forward with it. In the past, the response has been for us to submit a PR, and right now given I'm the only one of us maintaining polished, I don't have the time to also troubleshoot tsgen in order to get items out.

Right now this is not in the production build, so open to a discussion on how to move forward, but as of now I'm leaving it out so I can get things moving forward towards 2.0.

@ForbesLindesay
Copy link
Contributor

#250 is a pain for me to solve as I'll have to set up a windows VM to reproduce, but I can have a go if it's truly breaking.

I will add a config option to resolve #276 - I was using the allow synthetic default export option for my typescript app. If you were using the default transform to ES Modules this would work fine anyway, but you're compiling to CommonJS style.

I'll fix the tuples issue, I didn't know it was a big enough problem to lead to you dropping the library on polished.

@bhough
Copy link
Contributor Author

bhough commented Feb 9, 2018

On #250 it has prevented a few contributors from contributing since they are on Windows. We did a bunch of work to ensure the whole build worked in Windows for them, so this resulted in a regression for them.

On #276 does that mean only one or the other will work for typescript apps depending on how they have theirs configured?

I'm planning on going back through the flow defs as part of 2.0 and provide a lot stricter type safety. Tuples are a huge way of doing this for us, since we have a lot of modules with multiple api signatures.

Very much appreciate the help sorting these issues out.

@ForbesLindesay
Copy link
Contributor

On #276 does that mean only one or the other will work for typescript apps depending on how they have theirs configured?

No, the current type definitions are incorrect. Unlike flow, typescript differentiates between export default and module.exports =. There is a special mode that lets you treat them as equivalent, but if we get the library definitions correct, it won't matter which mode the consumer is using. Since you are doing module.exports = you'll need to put tsgen in the "Common JS Default Export" mode that is I'm about to add.

@ForbesLindesay
Copy link
Contributor

tsgen 1.1.0 with the --common-js-default-export flag should fix everything apart from the windows support.

@ForbesLindesay
Copy link
Contributor

Version 1.1.1 should slightly improve the error message, but I can't think of anything that would be causing this to break. Is this sufficient for you to re-introduce typescript definitions?

@Ponjimon
Copy link

Ponjimon commented Feb 10, 2018

If I can leave my bits here.

I noticed in my own project that:
"build:flow": "flow-copy-source -v -i '{**/*.spec.js}' src lib"
is not ignoring .spec.js files as it should be, they are still converted to .js.flow files.

If I change it to:
"build:flow": "flow-copy-source -v -i **/*.spec.js src lib"
then they are successfully ignored and then the build:typescript task should also run through.

That seems to be contrary to what the author of that lib said, but that's also what I noticed.
Macil/flow-copy-source#2

@resir014
Copy link

Hey, I'm in favour of bringing TypeScript back. Lack of typings will definitely prevent me from upgrading to 2.0 in my projects. Let me know how I could be of any help.

@bhough
Copy link
Contributor Author

bhough commented Feb 27, 2018

@resir014 I have a branch where I'm looking at @ForbesLindesay's changes: https://github.com/styled-components/polished/tree/readd-tsgen if you want to track progress on keeping it in 2.0.

bhough added a commit that referenced this pull request Mar 29, 2018
Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250
bhough added a commit that referenced this pull request Mar 29, 2018
Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250
bhough added a commit that referenced this pull request Mar 29, 2018
* build(typescript): Removes typescript and tsgen (#295)

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250

* feat(cover): Adds mixin for cover.

Adds mixin that provides css for fully covering an area, including an optional offset that acts as a
padding.
bhough added a commit that referenced this pull request Jun 1, 2018
Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250
bhough added a commit that referenced this pull request Jun 1, 2018
Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250
bhough added a commit that referenced this pull request Jun 1, 2018
* build(typescript): Removes typescript and tsgen (#295)

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250

* feat(cover): Adds mixin for cover.

Adds mixin that provides css for fully covering an area, including an optional offset that acts as a
padding.
bhough added a commit that referenced this pull request Jun 24, 2018
Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250
bhough added a commit that referenced this pull request Jun 24, 2018
* build(typescript): Removes typescript and tsgen (#295)

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250

* feat(cover): Adds mixin for cover.

Adds mixin that provides css for fully covering an area, including an optional offset that acts as a
padding.
bhough added a commit that referenced this pull request Aug 22, 2018
* build(typescript): Removes typescript and tsgen (#295)

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250

* build(docs): Remove doc autogeneration (#296)

Remove doc site and doc autogeneration from build to move to separate repo.

Fix #229

* feat(transitions): Same transition to multiple properties (#238)

* feat(transitions): Transitions can now apply the same values to multiple properties.

* fix(tests): Update snapshot and linting

* feat(border): Border shorthand (#297)

Border shorthand for splitting out individual border properties. Especially useful for atmoic css
libraries like Fela and Styletron.

Addresses #264

* fix(ellipsis): Harden ellipsis mixin. (#298)

Add `display: inline-flex` for browsers that support it better handle white-space issues with
`display: inline-block`

Addresses #256

* feat(fontFace): Add missing font-face properties to mixin (#299)

* feat(fontFace): Add missing font-face properties to mixin

Add font-display, font-feature-settings & font-variation-settings to fontDisplay mixin

closes #286

* chore(fontFace): Update fontFace snapshot

* fix(color): Color modules now support string amounts (#300)

All color modules now support string inputs for their amounts(percentage, amount, etc..) and
automatically convert them to ints/floats

Fix #243

* build(typescript): Removes typescript and tsgen (#295)

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250

* build(docs): Remove doc autogeneration (#296)

Remove doc site and doc autogeneration from build to move to separate repo.

Fix #229

* feat(border): Border shorthand (#297)

Border shorthand for splitting out individual border properties. Especially useful for atmoic css
libraries like Fela and Styletron.

Addresses #264

* feat(cover): Adds mixin for cover. (#304)

* build(typescript): Removes typescript and tsgen (#295)

Removes typescript tests and tsgen auto typing

BREAKING CHANGE: Anyone using typescript will lose typings with this release.

fix #276 #250

* feat(cover): Adds mixin for cover.

Adds mixin that provides css for fully covering an area, including an optional offset that acts as a
padding.

* feat(triangle): Add ability to use any units instead of only px to triangle mixin (#314)

* Fix link url to contributors' page (#307)

Just noted that link goes wrong, while have meeting with lib, fixed with a cap of tea :)

* docs(babel): add plugin (#308)

* Revert "docs(babel): add plugin (#308)" (#309)

This reverts commit 6083967.

* docs(Roadmap): Update ROADMAP.md to reflect current plan

* docs(Roadmap.md): Fixes for Roadmap.md

* feat(triangle): Add ability to use any units instead of only px to triangle mixin

closes #313

* refactor(triangle): Remove important from css declaration

BREAKING CHANGE: No longer adds important declaration

closes #313

* chore(normalize): Upgrade to normalize 8.0 (#316)

Upgrade to normalize.css 8.0 and fix bug with abbr[title] fallback.

re #315

*  feat(color): Add support for 8 CSS hexcolor values (#RRGGBBAA) (#318)

* feat(color): Add support for 8 CSS hexcolor values (#RRGGBBAA)

* chore(color): Color 8-Digit Hex Tests

Update tests for color modules to test more 8-Digit Hex cases.

* chore(Color): Properly round 8-Digit Hex

Properly round alpha values returned when processing 8-Digit Hex colors.

* chore(colors): Update 8-Digit tests

Updated 8-Digit Hex tests to ensure they match associated rgba values.

* [WIP]: feat(fluidRange): Add css-lock inspired fluidRange (#169)

* feat(fluidRange): Add css-lock inspired fluidRange

Adds fluidRange mixin that allows the user to leverage css-locks.

* feat(fluidRange): Improved fluidRange

* feat(fluidRange): fluidRange

* chore(fluidRange): more fluidRange work

* chore(fluidRange): Separate between from fluidRange

* chore(fluidRange): Update tests for fluidRange

* Update Dependencies (#319)

* chore(deps): Upgrade safe deps

* chore(deps): Upgrade warning deps

* chore(deps): Upgrade unsafe deps

Upgrade unsafe deps except for eslint 5

* chore(eslint): Upgrade eslint

Upgrade eslint and fix rollup config

* chore(types): Fix flow types (#320)

Fix issues with flow types from module changes and flow upgrade.

* chore(TypeScript): Re-enable tsgen (#321)

* chore(TypeScript): Re-enable tsgen

Re-enabled tsgen to generate TypeScript bindings.

fix #276

* build(build): Use yarn instead of npm

* test(transitions): Add error test to transitions

* Upgrade documentation.js (#322)

* docs(documentation.js): Upgrade documentation.js

Upgraded to latest release of documentation.js including fixing/updating the build.

* docs(Docs): Minor doc build fixes

Minor doc template fixes, move build away from `new Buffer`, and update toc.

* docs(docs): Cleanup docs config file

* fix(mix): Fix first param issues with mix. (#327)

Introducing a breaking change with mix that now requires the weight in order to function. The
previous version that tried to make the first param optional with a fallback broke with Babel
compilation and currying, and also did not provide the original intended API.

BREAKING CHANGE: Mix no longer has a default weight, it must always be provided.

* fix(index): Added missing modules to index.js (#328)

* feat(Flow): Define, use & document Styles type for mixins result values

* fix(Flow): Fix typo in Styles definition

* fix(Flow): Make mergeRules param type more specific to avoid type error

* fix(Typescript): Update tsgen to ^1.3 for index signature support

* chore(normalize): Update prefixed properties to camel case.

Fix #332

* chore(Flow): Fix flow errors with fluid range

* Bug Fixes (#334)

* chore(normalize): Update prefixed properties to camel case.

Fix #332

* chore(Flow): Fix flow errors with fluid range

* chore(Package): Update dependencies

* chore(docs): Docs review (#336)

* test(typeScript): Add additional TS tests (#337)

* feat(library): Deprecation warning support (#338)

* Alpha release 0 (#339)

* feat(library): Deprecation warning support

* build(package.json): Add alpha versionining to package.json

* Alpha release 1 (#340)

* feat(library): Deprecation warning support

* docs(docs): Updated Roadmap and Contributing, Travis config.

* fix(Travis): Move to single node version build

* Beta release 0 (#340) (#341)

* Alpha release 1 (#340)

* feat(library): Deprecation warning support

* docs(docs): Updated Roadmap and Contributing, Travis config.

* fix(Travis): Move to single node version build

* fix(Typescript): Fix typescript test for normalize

* fix(Multiple): Fix rounding, update dependencies, fix retinaImage

Adds rounding to several color mixins, fixes a bug with retinaImage that was causing backgroundSize
to be set as undefined, and updates deps.

* chore(package): Update deps, bump version for release
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.

None yet

5 participants