Skip to content

Conversation

@givanse
Copy link
Contributor

@givanse givanse commented Dec 30, 2018

This PRs does the following:

  • convert to TypeScript pretender.js => src/index.ts
    • the rollup config outputs the files:
      • pretender.js - this is the IIFE version, it should be working the same as before
      • pretender.es.js - this one is new
  • replace jscs with eslint (jscs merged with eslint)
  • bump the supported node versions
  • started using Yarn
  • depends on add module attribute FakeXMLHttpRequest#46

I'm still testing this with an actual app, but I think its ready to get reviewed and get a conversation started. done with that

{
"name": "pretender",
"version": "2.1.0",
"version": "3.1.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took the liberty of bumping the version number since this is a fairly large change to the code base.

* fullpath: '/mypage?test=yes'
* }
*/
function parseURL(url: string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TS is working

@@ -0,0 +1,48 @@
import PretenderES from '../src/pretender.es';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretender imported as a ES6 module. Also not that this test file is a ts file.

@givanse givanse changed the title Add a build step and produce ES and CJS files. Add a build step and produce ES6 files. Jan 2, 2019
const server = new Pretender(function() {});
```
Full example: [use-pretender-as-a-module](https://github.com/givanse/use-pretender-as-a-module)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the repo linked here, I added as collaborators the people that have merge access to Pretender.

Or if you prefer me to delete the mention of 'use-pretender-as-a-module', that is fine, I will.

@mike-north
Copy link
Member

mike-north commented Jan 2, 2019

Is there any reason why all of these

  • convert to TypeScript pretender.js => src/index.ts
    • the rollup config outputs the files:
      • pretender.js - this is the IIFE version, it should be working the same as before
      • pretender.es.js - this one is new
  • replace jscs with eslint (jscs merged with eslint)
  • bump the supported node versions
  • started using Yarn

must be included in a single PR? I could see this being at least 4

  1. use yarn
  2. jscs -> eslint
  3. es module conversion (changing existing tests as little as possible, in order to prove that this is purely a packaging change)
  4. ts conversion (it's nearly always a better idea to do this as an independent PR, to ensure that the code change only adds type information without modifying runtime behavior in any breaking way)

This also has the potential to be a minor release.

@givanse
Copy link
Contributor Author

givanse commented Jan 2, 2019

you are right, this got too messy, I'll split it up

@givanse givanse closed this Jan 2, 2019
@givanse
Copy link
Contributor Author

givanse commented Jan 2, 2019

PR #243

Add build step and TS support

@givanse givanse deleted the rollup branch January 2, 2019 18:03
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