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

Update history and imports in test code #2613

Closed
wants to merge 1 commit into from

Conversation

toranb
Copy link

@toranb toranb commented Nov 30, 2015

I wanted to update the imports I found in test code (and the dependency itself)

@taion
Copy link
Contributor

taion commented Nov 30, 2015

Per https://github.com/rackt/history/blob/master/docs/GettingStarted.md#minimizing-your-build you actually shouldn't import history that way, so I believe what we have there is intentional.

We also don't really get anything from updating the dev dependency since the semver range still matches the current version.

@toranb
Copy link
Author

toranb commented Nov 30, 2015

Understood - quick comment about the link you sent over => it seems to be about minimizing your build size (is this required/ the most important thing for test code) ?

@taion
Copy link
Contributor

taion commented Nov 30, 2015

It doesn't matter at all for test code - it's just defensive against people seeing us use a particular pattern in test code and copying it in their own code.

Same reason we do e.g. https://github.com/rackt/react-router/blob/4bea5c1143c44cc5b4019d0954627696b3ff0d70/modules/__tests__/Router-test.js#L288. That's an awful way to write that code v. binding in render or something, but it's just defensive as a potential example.

The only time you can safely import stuff like that is when you're using an ES6-module-aware bundler that can do tree shaking... this is basically just rollup (not really suitable for applications) or webpack 2 (last I checked babel-loader didn't work there yet, so not really suitable for React anything).

And only for packages that have an ES6 module build... which history will on the next release assuming my change adding that build doesn't get backed out per #2530 (comment).

You okay with closing this?

@toranb toranb closed this Nov 30, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 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

Successfully merging this pull request may close these issues.

2 participants