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

Bump RWS and RWS interfaces #1041

Merged
merged 1 commit into from
Feb 23, 2018
Merged

Bump RWS and RWS interfaces #1041

merged 1 commit into from
Feb 23, 2018

Conversation

majapw
Copy link
Collaborator

@majapw majapw commented Feb 23, 2018

I have tested this by running npm run build and npm run build:css and both seem to successfully generate the CSS file! :) This will allow us to not have this weird deps issue and get the latest and greatest of react-with-styles changes.

to: @ljharb @gabergg

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.513% when pulling f953b03 on maja-bump-rws-and-rws-css into c22ccc2 on master.

Copy link
Collaborator

@gabergg gabergg left a comment

Choose a reason for hiding this comment

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

LGTM

@majapw majapw merged commit f5580ac into master Feb 23, 2018
@majapw majapw deleted the maja-bump-rws-and-rws-css branch February 23, 2018 21:14
@aminland
Copy link

aminland commented Feb 24, 2018

This change caused our webpack builds to start failing with the following error (v16.3.0 was fine):

Module not found: Error: Recursion in resolving
Stack:
  resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten
  new-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten
  parsed-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten module
  described-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten module
  raw-module: (/Volumes/Documents/projects/fellow-core/web/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten
  module: (/Volumes/Documents/projects/fellow-core/web/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten
  resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules) ./array.prototype.flatten
  new-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules) ./array.prototype.flatten
  parsed-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules) ./array.prototype.flatten
  described-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules) ./array.prototype.flatten
  relative: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) 
  described-relative: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) 
  directory: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) 
  existing-directory: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) 
  resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) ./
  new-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) ./
  parsed-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) . directory
  described-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) . directory
  relative: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten)  directory
  described-relative: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten)  directory
  directory: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten)  directory
  existing-directory: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten)  directory
  resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) ./ directory
  new-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) ./ directory
  parsed-resolve: (/Volumes/Documents/projects/fellow-core/web/node_modules/array.prototype.flatten) . directory
 @ ./node_modules/react-with-styles-interface-css/dist/index.js 5:22-56
 @ ./node_modules/react-with-styles-interface-css/index.js
 @ ./node_modules/react-dates/lib/utils/registerCSSInterfaceWithDefaultTheme.js
 @ ./node_modules/react-dates/lib/initialize.js
 @ ./node_modules/react-dates/initialize.js
 @ ./src/components/App.js
 @ ./src/index.js
 @ multi (webpack)-dev-server/client?http://0.0.0.0 webpack/hot/only-dev-server react-hot-loader/patch babel-polyfill ./src/index.cjsx

All we're doing is an import at the top of our App.js component file import 'react-dates/initialize';

I'm pretty sure this is a significant backwards-incompatible change, and shouldn't have been released as a "patch".

@ianshea
Copy link

ianshea commented Feb 24, 2018

I'm having issues with this as well. I whipped a basic example.

https://github.com/ianshea/react-dates-broken-example

Adding array.prototype.flatten as a dependency with npm install --save array.prototype.flatten in the project does not alleviate this issue.

@majapw
Copy link
Collaborator Author

majapw commented Feb 24, 2018 via email

@majapw
Copy link
Collaborator Author

majapw commented Feb 24, 2018

Hi @aminland @ianshea can you check to see if this is still an issue. I just released v16.3.2 with the revert. Sorry for the inconvenience. D:

@ianshea
Copy link

ianshea commented Feb 24, 2018

It's all good! I just did an update and reran my demo and I'm still seeing the issue though.

Specifically from the next command npm run dev I see the following:

 ERROR  Failed to compile with 1 errors                                                                                                                   18:45:55

This dependency was not found:

* array.prototype.flatten in ./node_modules/react-with-styles-interface-css/dist/index.js

To install it, you can run: npm install --save array.prototype.flatten

And here's a big ol' stack trace I'm seeing.

Error in ./dist/index.js
Module not found: Error: Recursion in resolving Stack: resolve: (/Users/tester/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten new-resolve: (/Users/tester/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten parsed-resolve: (/Users/tester/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten module described-resolve: (/Users/tester/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten module raw-module: (/Users/tester/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten module: (/Users/tester/node_modules/react-with-styles-interface-css/dist) array.prototype.flatten resolve: (/Users/tester/node_modules) ./array.prototype.flatten new-resolve: (/Users/tester/node_modules) ./array.prototype.flatten parsed-resolve: (/Users/tester/node_modules) ./array.prototype.flatten described-resolve: (/Users/tester/node_modules) ./array.prototype.flatten relative: (/Users/tester/node_modules/array.prototype.flatten) described-relative: (/Users/tester/node_modules/array.prototype.flatten) directory: (/Users/tester/node_modules/array.prototype.flatten) existing-directory: (/Users/tester/node_modules/array.prototype.flatten) resolve: (/Users/tester/node_modules/array.prototype.flatten) ./ new-resolve: (/Users/tester/node_modules/array.prototype.flatten) ./ parsed-resolve: (/Users/tester/node_modules/array.prototype.flatten) . directory described-resolve: (/Users/tester/node_modules/array.prototype.flatten) . directory relative: (/Users/tester/node_modules/array.prototype.flatten) directory described-relative: (/Users/tester/node_modules/array.prototype.flatten) directory directory: (/Users/tester/node_modules/array.prototype.flatten) directory existing-directory: (/Users/tester/node_modules/array.prototype.flatten) directory resolve: (/Users/tester/node_modules/array.prototype.flatten) ./ directory new-resolve: (/Users/tester/node_modules/array.prototype.flatten) ./ directory parsed-resolve: (/Users/tester/node_modules/array.prototype.flatten) . directory

@majapw
Copy link
Collaborator Author

majapw commented Feb 24, 2018 via email

@aminland
Copy link

I can confirm it is still broken, even after removing package-lock, node_modules, and fresh npm install.

manually switching installed "react-with-styles-interface-css" to "^3.0.0" fixes the problem for me locally.

@@ -120,7 +120,7 @@
"react-addons-shallow-compare": "^15.6.2",
"react-moment-proptypes": "^1.5.0",
"react-portal": "^4.1.2",
"react-with-styles": "=2.2.0",
"react-with-styles": "^3.1.0",
"react-with-styles-interface-css": "^4.0.0"

Choose a reason for hiding this comment

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

This should be ^3.0.0 to resolve import error

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So it's not actually this PR that is the issue. It's #1029

Copy link
Collaborator Author

@majapw majapw Feb 24, 2018

Choose a reason for hiding this comment

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

Okay v16.3.3 is out and should address this. I'm gonna investigate what the heck is going on.

@aminland
Copy link

So off of version 16.3.1, i could get it to work by changing the "main" entry in array.prototype.flatten/package.json to "index.js"

it is currently "./"

I haven't seen that anywhere before, not sure if it's a standard thing, or maybe it's my node version (8.9.2), but it seems to be the root cause. I believe you guys switched to using that shim from array_flatten in the react-with-styles-interface-css update.

@ljharb
Copy link
Member

ljharb commented Feb 24, 2018

It’s definitely a standard thing; maybe it’s a webpack bug?

Could you share your webpack versions and config?

@aminland
Copy link

It's a webpack bug: webpack/enhanced-resolve#123

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.

None yet

6 participants