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

feat: update react-dnd #531

Merged
merged 8 commits into from
Oct 14, 2019
Merged

Conversation

kserjey
Copy link
Contributor

@kserjey kserjey commented Sep 13, 2019

No description provided.

@dukoo
Copy link

dukoo commented Sep 13, 2019

After upgrading to react-dnd 9.3.4 sortable tree without dnd context is not working. So I'm looking forward to see the update comming soon :)

@hellofantastic
Copy link

Does this finish up #507 and ring in a sortable tree 2.6.3?

@kserjey
Copy link
Contributor Author

kserjey commented Sep 16, 2019

I fixed all issues with jest and added prepare script to install my fork from github (but it still doesn't work). All test have passed, so I think it could be merged into master and released as 2.6.3.

@coveralls
Copy link

coveralls commented Sep 16, 2019

Coverage Status

Coverage increased (+0.1%) to 75.702% when pulling 4d98d56 on kserjey:master into 420d602 on frontend-collective:master.

@OliverKK
Copy link

OliverKK commented Sep 19, 2019

Hello @fritz-c && @wuweiweiwu ,
first of all thanks for this awesome tree component.

Are you thinking about developing the library further and accepting PullRequests?
It's been quieter here for a while :)

@kserjey
Copy link
Contributor Author

kserjey commented Sep 19, 2019

If someone want working react-sortable-tree with react-dnd@9, you can install if from my fork with postinstall script:

"scripts": {
+ "postinstall": "yarn --cwd ./node_modules/react-sortable-tree run buildOnly",
},
"dependencies": {
- "react-sortable-tree": "2.6.2",
+ "react-sortable-tree": "github:kserjey/react-sortable-tree",
},
"devDependencies": {
+ "rollup": "^1.21.4",
+ "rollup-plugin-babel": "^4.3.3",
+ "rollup-plugin-commonjs": "^10.1.0",
+ "rollup-plugin-node-resolve": "^5.2.0",
+ "rollup-plugin-postcss": "^2.0.3",
}

@lifejuggler lifejuggler merged commit c449524 into frontend-collective:master Oct 14, 2019
@lifejuggler
Copy link
Collaborator

merged! Thank you :D

@shiraze
Copy link

shiraze commented Nov 15, 2019

@lifejuggler , is there any reason we jumped from dnd@7.7.0 to dnd@9.4.0?
My tests are failing with a cryptic message:

Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    C:\Source\React\react\node_modules\react-dnd\dist\esm\index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){export * from './common';
                                                                                             ^^^^^^

    SyntaxError: Unexpected token export

      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
      at Object.<anonymous> (node_modules/frontend-collective-react-dnd-scrollzone/lib/index.js:24:17)

@lifejuggler
Copy link
Collaborator

lifejuggler commented Nov 15, 2019

@shiraze this was done in part to modernize the library and potentially deal with security vulnerabilities that might comeup with having an outdated library. if you had ecountered the failing test can you write a ticket for it and I will try to investigate when I have the time... which now a days seems to be almost nonexistant... if this is a big problem I can also revert the merge until all the issues have been addressed

@shiraze
Copy link

shiraze commented Nov 15, 2019

@lifejuggler , we've got around the issue by updating our Jest config to include:

    "transformIgnorePatterns": [
      "node_modules/(?!(react-dnd|frontend-collective-react-dnd-scrollzone|react-sortable-tree|dnd-core|react-dnd-html5-backend)/)"
    ]

I'm not sure why Jest is attempting to test with files in the esm (non-transpiled) folder rather than the cjs (transpiled) folder.
The failures are just stating that "import" (from index.js) is not recognised.
Prior to this update jest was picking up the correct files.

I'd only consider a revert if others are stuck (although they can do what I did!)

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.

7 participants