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

Use named import of 'history' module to fix #5576. #5589

Merged
merged 2 commits into from Nov 13, 2017
Merged

Use named import of 'history' module to fix #5576. #5589

merged 2 commits into from Nov 13, 2017

Conversation

RoboBurned
Copy link
Contributor

Fixes #5576
The issue is when we have something like
import createBrowserHistory from 'history/createBrowserHistory'
you get file included into the result dist:
node_modules/history/createBrowserHistory.js
And then when you import something like
import {smth} from 'history'
and your webpack or other build tool is configured to read packages.json>module property then it includes node_modules/history/es/index.js and then node_modules/history/es/createBrowserHistory.js
So both files are included:

node_modules/history/createBrowserHistory.js
node_modules/history/es/createBrowserHistory.js

@remix-run remix-run deleted a comment from DevSide Oct 11, 2017
Copy link
Contributor

@pshrmn pshrmn left a comment

Choose a reason for hiding this comment

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

This looks fine to me.

@RoboBurned I assume that you have built a project with these changes and it only included the history code you expected it to, yes?

This will also break cherry picking (I assume that is why this was written this way), but I'm not sure if we are still supporting that anyways.

Copy link
Member

@timdorr timdorr left a comment

Choose a reason for hiding this comment

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

Sorry, forgot to mention this, but the import brackets should have spaces on the inside for consistence. E.g.:

import { createBrowserHistory as createHistory } from 'history'

@pshrmn pshrmn mentioned this pull request Nov 11, 2017
@RoboBurned
Copy link
Contributor Author

@pshrmn, yes, after these changes project includes only one copy of code from history/es directory.
@timdorr, I've added missing spaces.

@timdorr
Copy link
Member

timdorr commented Nov 13, 2017

Thanks!

@timdorr timdorr merged commit a3e6fac into remix-run:master Nov 13, 2017
bors bot added a commit to mozilla/delivery-console that referenced this pull request Jun 6, 2018
179: Update dependency react-router-dom to v4.3.0 r=magopian a=renovate[bot]

This Pull Request updates dependency [react-router-dom](https://github.com/ReactTraining/react-router) from `v4.2.2` to `v4.3.0`



<details>
<summary>Release Notes</summary>

### [`v4.3.0`](https://github.com/ReactTraining/react-router/blob/master/CHANGELOG.md#v430httpsgithubcomReactTrainingreact-routercomparev420v430)
[Compare Source](remix-run/react-router@v4.3.0-rc.3...a27bc56)
> Jun 6, 2018

* Use the `pretty` option in generatePath ([#&#8203;6172] by @&#8203;sibelius)
* aria-current has incorrect value "true" ([#&#8203;6118] by @&#8203;brandonrninefive)
* Redirect with parameters ([#&#8203;5209] by @&#8203;dlindenkreuz)
* Fix with missing pathname: `<Link to="?foo=bar">` ([#&#8203;5489] by @&#8203;pshrmn)
* Escape NavLink path to allow special characters in path. ([#&#8203;5596] by @&#8203;esiegel)
* Expose `generatePath` ([#&#8203;5661] by @&#8203;rybon)
* Use named import of history module. ([#&#8203;5589] by @&#8203;RoboBurned)
* Hoist dependencies for smaller UMD builds ([#&#8203;5720] by @&#8203;pshrmn)
* Remove aria-current from navLink when inactive ([#&#8203;5508] by @&#8203;AlmeroSteyn)
* Add invariant for missing "to" property on `<Link>` ([#&#8203;5792] by @&#8203;selbekk)
* Use Prettier on the code ([e6f9017] by @&#8203;mjackson)
* Fix pathless route's match when parent is null ([#&#8203;5964] by @&#8203;pshrmn)
* Use history.createLocation in `<StaticRouter>` ([#&#8203;5722] by @&#8203;pshrmn)

[#&#8203;6172]: `remix-run/react-router#6172
[#&#8203;6118]: `remix-run/react-router#6118
[#&#8203;5209]: `remix-run/react-router#5209
[#&#8203;5489]: `remix-run/react-router#5489
[#&#8203;5596]: `remix-run/react-router#5596
[#&#8203;5661]: `remix-run/react-router#5661
[#&#8203;5589]: `remix-run/react-router#5589
[#&#8203;5720]: `remix-run/react-router#5720
[#&#8203;5508]: `remix-run/react-router#5508
[#&#8203;5792]: `remix-run/react-router#5792
[e6f9017]: remix-run/react-router@e6f9017
[#&#8203;5964]: `remix-run/react-router#5964
[#&#8203;5722]: `remix-run/react-router#5722

---

### [`v4.3.0-rc.3`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.3)
[Compare Source](remix-run/react-router@v4.3.0-rc.2...v4.3.0-rc.3)
- Fix broken UMD builds.
- Add sideEffects: false for webpack tree shaking (#&#8203;6082 by @&#8203;taylorc93)

---

### [`v4.3.0-rc.2`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.2)
[Compare Source](remix-run/react-router@v4.3.0-rc.1...v4.3.0-rc.2)
- Bump `hoist-non-react-statics` for React 16.3.
- Missing `generatePath` in `react-router-dom` package.

---

### [`v4.3.0-rc.1`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.1)
[Compare Source](remix-run/react-router@v4.2.2...v4.3.0-rc.1)
> Mar 26, 2018

- Redirect with parameters ([#&#8203;5209] by @&#8203;dlindenkreuz)
- Fix with missing pathname: `<Link to="?foo=bar">` ([#&#8203;5489] by @&#8203;pshrmn)
- Escape NavLink path to allow special characters in path. ([#&#8203;5596] by @&#8203;esiegel)
- Expose `generatePath` ([#&#8203;5661] by @&#8203;rybon)
- Use named import of history module. ([#&#8203;5589] by @&#8203;RoboBurned)
- Hoist dependencies for smaller UMD builds ([#&#8203;5720] by @&#8203;pshrmn)
- Remove aria-current from navLink when inactive ([#&#8203;5508] by @&#8203;AlmeroSteyn)
- Add invariant for missing "to" property on `<Link>` ([#&#8203;5792] by @&#8203;selbekk)
- Use Prettier on the code ([e6f9017] by @&#8203;mjackson)
- Fix pathless route's match when parent is null ([#&#8203;5964] by @&#8203;pshrmn)
- Use history.createLocation in `<StaticRouter>` ([#&#8203;5722] by @&#8203;pshrmn)

[#&#8203;5209]: `remix-run/react-router#5209
[#&#8203;5489]: `remix-run/react-router#5489
[#&#8203;5596]: `remix-run/react-router#5596
[#&#8203;5661]: `remix-run/react-router#5661
[#&#8203;5589]: `remix-run/react-router#5589
[#&#8203;5720]: `remix-run/react-router#5720
[#&#8203;5508]: `remix-run/react-router#5508
[#&#8203;5792]: `remix-run/react-router#5792
[e6f9017]: remix-run/react-router@e6f9017
[#&#8203;5964]: `remix-run/react-router#5964
[#&#8203;5722]: `remix-run/react-router#5722

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).

Co-authored-by: Renovate Bot <bot@renovateapp.com>
bors bot added a commit to mozilla/delivery-console that referenced this pull request Jun 6, 2018
178: Update dependency react-router to v4.3.0 r=magopian a=renovate[bot]

This Pull Request updates dependency [react-router](https://github.com/ReactTraining/react-router) from `v4.2.0` to `v4.3.0`



<details>
<summary>Release Notes</summary>

### [`v4.3.0`](https://github.com/ReactTraining/react-router/blob/master/CHANGELOG.md#v430httpsgithubcomReactTrainingreact-routercomparev420v430)
[Compare Source](remix-run/react-router@v4.3.0-rc.3...v4.3.0)
> Jun 6, 2018

* Use the `pretty` option in generatePath ([#&#8203;6172] by @&#8203;sibelius)
* aria-current has incorrect value "true" ([#&#8203;6118] by @&#8203;brandonrninefive)
* Redirect with parameters ([#&#8203;5209] by @&#8203;dlindenkreuz)
* Fix with missing pathname: `<Link to="?foo=bar">` ([#&#8203;5489] by @&#8203;pshrmn)
* Escape NavLink path to allow special characters in path. ([#&#8203;5596] by @&#8203;esiegel)
* Expose `generatePath` ([#&#8203;5661] by @&#8203;rybon)
* Use named import of history module. ([#&#8203;5589] by @&#8203;RoboBurned)
* Hoist dependencies for smaller UMD builds ([#&#8203;5720] by @&#8203;pshrmn)
* Remove aria-current from navLink when inactive ([#&#8203;5508] by @&#8203;AlmeroSteyn)
* Add invariant for missing "to" property on `<Link>` ([#&#8203;5792] by @&#8203;selbekk)
* Use Prettier on the code ([e6f9017] by @&#8203;mjackson)
* Fix pathless route's match when parent is null ([#&#8203;5964] by @&#8203;pshrmn)
* Use history.createLocation in `<StaticRouter>` ([#&#8203;5722] by @&#8203;pshrmn)

[#&#8203;6172]: `remix-run/react-router#6172
[#&#8203;6118]: `remix-run/react-router#6118
[#&#8203;5209]: `remix-run/react-router#5209
[#&#8203;5489]: `remix-run/react-router#5489
[#&#8203;5596]: `remix-run/react-router#5596
[#&#8203;5661]: `remix-run/react-router#5661
[#&#8203;5589]: `remix-run/react-router#5589
[#&#8203;5720]: `remix-run/react-router#5720
[#&#8203;5508]: `remix-run/react-router#5508
[#&#8203;5792]: `remix-run/react-router#5792
[e6f9017]: remix-run/react-router@e6f9017
[#&#8203;5964]: `remix-run/react-router#5964
[#&#8203;5722]: `remix-run/react-router#5722

---

### [`v4.3.0-rc.3`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.3)
[Compare Source](remix-run/react-router@v4.3.0-rc.2...v4.3.0-rc.3)
- Fix broken UMD builds.
- Add sideEffects: false for webpack tree shaking (#&#8203;6082 by @&#8203;taylorc93)

---

### [`v4.3.0-rc.2`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.2)
[Compare Source](remix-run/react-router@v4.3.0-rc.1...v4.3.0-rc.2)
- Bump `hoist-non-react-statics` for React 16.3.
- Missing `generatePath` in `react-router-dom` package.

---

### [`v4.3.0-rc.1`](https://github.com/ReactTraining/react-router/releases/v4.3.0-rc.1)
[Compare Source](remix-run/react-router@v4.2.0...v4.3.0-rc.1)
> Mar 26, 2018

- Redirect with parameters ([#&#8203;5209] by @&#8203;dlindenkreuz)
- Fix with missing pathname: `<Link to="?foo=bar">` ([#&#8203;5489] by @&#8203;pshrmn)
- Escape NavLink path to allow special characters in path. ([#&#8203;5596] by @&#8203;esiegel)
- Expose `generatePath` ([#&#8203;5661] by @&#8203;rybon)
- Use named import of history module. ([#&#8203;5589] by @&#8203;RoboBurned)
- Hoist dependencies for smaller UMD builds ([#&#8203;5720] by @&#8203;pshrmn)
- Remove aria-current from navLink when inactive ([#&#8203;5508] by @&#8203;AlmeroSteyn)
- Add invariant for missing "to" property on `<Link>` ([#&#8203;5792] by @&#8203;selbekk)
- Use Prettier on the code ([e6f9017] by @&#8203;mjackson)
- Fix pathless route's match when parent is null ([#&#8203;5964] by @&#8203;pshrmn)
- Use history.createLocation in `<StaticRouter>` ([#&#8203;5722] by @&#8203;pshrmn)

[#&#8203;5209]: `remix-run/react-router#5209
[#&#8203;5489]: `remix-run/react-router#5489
[#&#8203;5596]: `remix-run/react-router#5596
[#&#8203;5661]: `remix-run/react-router#5661
[#&#8203;5589]: `remix-run/react-router#5589
[#&#8203;5720]: `remix-run/react-router#5720
[#&#8203;5508]: `remix-run/react-router#5508
[#&#8203;5792]: `remix-run/react-router#5792
[e6f9017]: remix-run/react-router@e6f9017
[#&#8203;5964]: `remix-run/react-router#5964
[#&#8203;5722]: `remix-run/react-router#5722

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).

Co-authored-by: Renovate Bot <bot@renovateapp.com>
lencioni added a commit to lencioni/react-router that referenced this pull request Aug 15, 2018
I was recently doing some bundle size analysis using source-map-explorer
and noticed that we had a little bit of duplication of react-router
code. It seems as though our import of react-router-config was bringing
in modules from react-router/Foo, while our named imports from
react-router was bringing in modules from react-router/es/Foo.

As a workaround, I've set up a webpack alias to avoid the duplication,
but I think this should be solved in this package so that everyone can
benefit.

I believe this is similar to:

- remix-run#5589
timdorr pushed a commit that referenced this pull request Aug 23, 2018
I was recently doing some bundle size analysis using source-map-explorer
and noticed that we had a little bit of duplication of react-router
code. It seems as though our import of react-router-config was bringing
in modules from react-router/Foo, while our named imports from
react-router was bringing in modules from react-router/es/Foo.

As a workaround, I've set up a webpack alias to avoid the duplication,
but I think this should be solved in this package so that everyone can
benefit.

I believe this is similar to:

- #5589
jeresig pushed a commit to Khan/react-router that referenced this pull request Aug 29, 2018
…n#5589)

* Use named import of 'history' module.

* Coding style fixes.
jeresig pushed a commit to Khan/react-router that referenced this pull request Aug 29, 2018
…#6291)

I was recently doing some bundle size analysis using source-map-explorer
and noticed that we had a little bit of duplication of react-router
code. It seems as though our import of react-router-config was bringing
in modules from react-router/Foo, while our named imports from
react-router was bringing in modules from react-router/es/Foo.

As a workaround, I've set up a webpack alias to avoid the duplication,
but I think this should be solved in this package so that everyone can
benefit.

I believe this is similar to:

- remix-run#5589
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
handholdinginc pushed a commit to handholdinginc/social that referenced this pull request Sep 6, 2021
It was adding 1 minute to build time, and the upstream issue is already fixed: remix-run/react-router#5589
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.

None yet

3 participants