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

fix: Fix imports to play nicely with rollup #530

Merged
merged 1 commit into from Aug 2, 2019
Merged

fix: Fix imports to play nicely with rollup #530

merged 1 commit into from Aug 2, 2019

Conversation

bencentra-toast
Copy link
Contributor

@bencentra-toast bencentra-toast commented Aug 2, 2019

Fixes #529, and a similar issues with ReactDOM.findDOMNode()

Before this change, I was getting errors with rolllup such as:

(!) Missing exports
https://rollupjs.org/guide/en#error-name-is-not-exported-by-module-
../react-transition-group/lib/esm/Transition.js
oneOfType is not exported by ../react-transition-group/node_modules/prop-types/index.js
419:    * ```
420:    */
421:   children: PropTypes.oneOfType([PropTypes.func.isRequired, PropTypes.element.isRequired]).isRequired,
                           ^
422:
423:   /**

and

[!] Error: 'findDOMNode' is not exported by ../react-transition-group/node_modules/react-dom/index.js
https://rollupjs.org/guide/en#error-name-is-not-exported-by-module-
../react-transition-group/lib/esm/ReplaceTransition.js (5:9)
3: import PropTypes from 'prop-types';
4: import React from 'react';
5: import { findDOMNode } from 'react-dom';
            ^
6: import TransitionGroup from './TransitionGroup';
7: /**
Error: 'findDOMNode' is not exported by ../react-transition-group/node_modules/react-dom/index.js
    at error (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:3598:30)
    at Module.error (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:14473:9)
    at handleMissingExport (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:14400:21)
    at Module.traceVariable (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:14742:17)
    at ModuleScope.findVariable (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:13444:37)
    at FunctionScope.ChildScope.findVariable (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:4281:67)
    at ChildScope.findVariable (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:4281:67)
    at FunctionScope.ChildScope.findVariable (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:4281:67)
    at ChildScope.findVariable (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:4281:67)
    at Identifier.bind (/Users/bencentra/toast/git-repos/buffet/node_modules/rollup/dist/rollup.js:11247:40)

After this change, I can run my project's rollup successfully ($ NODE_ENV=production rollup -c)

Tested locally by yarn link-ing react-transition-group to my project and running yarn build before building my own project.

I don't know exactly what (if anything) changed with this package that would've caused these issues, since the lines I'm changing are over a year old. react-dom got updated 2 days ago, prop-types 6 months ago, and rollup yesterday. Maybe it's a rollup problem?

@taion taion changed the title fix imports to play nicely with rollup fix: Fix imports to play nicely with rollup Aug 2, 2019
@taion taion merged commit 3d9003e into reactjs:master Aug 2, 2019
@bencentra-toast bencentra-toast deleted the import-fixes branch August 2, 2019 19:08
@InsaneViku
Copy link

Ah, glad you also got this @bencentra-toast, thanks!

jquense pushed a commit that referenced this pull request Aug 2, 2019
## [4.2.2](v4.2.1...v4.2.2) (2019-08-02)

### Bug Fixes

* Fix imports to play nicely with rollup ([#530](#530)) ([3d9003e](3d9003e))
@jquense
Copy link
Collaborator

jquense commented Aug 2, 2019

🎉 This PR is included in version 4.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@TrySound
Copy link
Contributor

TrySound commented Aug 3, 2019

You still could use it successfully. It's ok to use named imports. Here's workaround for your issue in rollup config

import * as React from 'react';
import * as ReactDOM from 'react-dom';
import * as PropTypes from 'prop-types';

export default {
  ...
  commons({
    ...,
    namedExports: {
      react: Object.keys(React),
      'react-dom': Object.keys(ReactDOM),
      'prop-types': Object.keys(PropTypes)
    }
  })
  ...
}

@InsaneViku
Copy link

@TrySound, thanks for the workaround - good to note for other cases similar to this that can pop up in the wild, until expected and consistent usage can be applied like this.

@TrySound
Copy link
Contributor

TrySound commented Aug 3, 2019

Well, named exports makes more sense in case of react. This workaround exist until react will provide esm support. Default export will probably be dropped eventually.

johnfrench3 pushed a commit to johnfrench3/transition-group-react that referenced this pull request Nov 2, 2022
## [4.2.2](reactjs/react-transition-group@v4.2.1...v4.2.2) (2019-08-02)

### Bug Fixes

* Fix imports to play nicely with rollup ([#530](reactjs/react-transition-group#530)) ([3d9003e](reactjs/react-transition-group@3d9003e))
patrickm68 added a commit to patrickm68/react-transition-group-developer that referenced this pull request Dec 1, 2022
## [4.2.2](reactjs/react-transition-group@v4.2.1...v4.2.2) (2019-08-02)

### Bug Fixes

* Fix imports to play nicely with rollup ([#530](reactjs/react-transition-group#530)) ([3d9003e](reactjs/react-transition-group@3d9003e))
shaikdev2 pushed a commit to shaikdev2/transition-group-react that referenced this pull request Jun 9, 2023
## [4.2.2](reactjs/react-transition-group@v4.2.1...v4.2.2) (2019-08-02)

### Bug Fixes

* Fix imports to play nicely with rollup ([#530](reactjs/react-transition-group#530)) ([3d9003e](reactjs/react-transition-group@3d9003e))
GreenCat1996 added a commit to GreenCat1996/react-transition-group that referenced this pull request Aug 1, 2023
## [4.2.2](reactjs/react-transition-group@v4.2.1...v4.2.2) (2019-08-02)

### Bug Fixes

* Fix imports to play nicely with rollup ([#530](reactjs/react-transition-group#530)) ([3d9003e](reactjs/react-transition-group@3d9003e))
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.

Transition and CSSTransition component files import prop-types incorrectly
5 participants