Skip to content

Conversation

spong
Copy link
Contributor

@spong spong commented Oct 21, 2019

This PR adds an ES5 distribution (with source map) to dist/web and sets the package.json browser field to point to it.

This keeps the previous typescript configuration and commonJS/ES Module distributions in tact, and provides a new build-web script that uses the Babel CLI to transpile from the ESM source.

Running npm run build will build all three distributions.

@spong spong mentioned this pull request Oct 21, 2019
Copy link
Member

@pimterry pimterry left a comment

Choose a reason for hiding this comment

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

Did you try doing this with just tsconfig? It seems that that would be simpler (just another extends file with the target & module format of choice), would skip a dependency, and would be more consistent with the other two formats.

The one other bit that surprised me a little was that it outputs backward compatible JS, but with ESM module syntax. I think there's an argument for UMD instead, but also nowadays I find that UMD tends to break things as much as it helps, since it's not as easily parseable. What's the reasoning there?

@spong
Copy link
Contributor Author

spong commented Oct 22, 2019

I had first tried using just the typescript compiler to keep things simple as you said, but was unable to get an ES5 distribution that IE11 would load and so reverted to a babel config I knew would work.

I just gave it another try though and was able to put together a tsconfig that works (PR updated with these changes). That said, using "module": "umd" results in IE11 erroring out with cannot find module 'react'. The only module setting I could get it to work with was "es6", so that's the rationale there.

As for naming (the es5 tsconfig & corresponding dist folder), not sure what's best here since you had them based by module type -- thoughts?

@pimterry
Copy link
Member

Thanks, with those changes this looks great to me. I think web is a good target name, that's fine.

I've updated the tsconfig name from es5 to web so that matches too - I'll merge now and release as 1.0.4 imminently. Let me know if you have any other trouble, thanks for contributing! 👍

@pimterry pimterry merged commit 4027b8a into httptoolkit:master Oct 23, 2019
@spong
Copy link
Contributor Author

spong commented Oct 23, 2019

Perfect -- thanks so much @pimterry! 🙂 I've updated to 1.0.4 and verified IE11 backwards compatibility + no issue in Chrome/FF/Safari, so all is good! 👍

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