search

facebook / fbjs

A collection of utility libraries used by other Meta JS projects.
MIT License
1.95k stars 313 forks source link

Request to upgrade core-js version to be ^2.x in the published version of fbjs #300

Closed karanpvyas closed 4 years ago

karanpvyas commented 6 years ago

with this change(fix) in core-js https://github.com/zloirock/core-js/commit/78770786a4e5f2f87ea32b4dada46b9ce0a92887 and it being available only with ^2.4.x of core-js, can the core-js version please be upgraded in fbjs?

Thanks!

zpao commented 6 years ago

I don't think I'll do that in v0.8.x. We're on ^1.0.0 and bumping to 2.x may cause issues. Also, AFAIK our usage is constrained to Map and Set libraries (not even polyfills) so shouldn't matter for your own uses.

Are you seeing problems today?

karanpvyas commented 6 years ago

Thanks for the prompt reply @zpao , the issue being: We've upgraded all possible libraries that we're using so that they depend on core-js@^2.4.x

But for those that use fbjs (@0.8.x) , it brings in core-js@^1.0.0, which causes the issue (thats fixed in the commit i mentioned in core-js)

P.S the exact issue faced is

Now because a lot of react-* libraries use fbjs@0.8.x, can you please advise on how to go about the issue, If core-js version can't be bumped up right now? Thanks!

karanpvyas commented 6 years ago

More details on the exact issue:

There was a bug in Symbol polyfill of core-js@(<2.4), when the polyfill was getting used (in IE where there is no native Symbol support) -> whenever a new Symbol would be created in our js code once the polyfill code is run, it would make it an enumerable property of Object's Prototype, so hence any new objects created would have the Symbol (just created) as an inherited enumerable property (which was wrong).

Now this caused issues in _pickBy and _omitBy kind of use cases, where in all own and inhertied enumerable properties are iterated over (or any such case where we would do so in our own code);

This was fixed in the commit I've mentioned above for core-js. But since fbjs relies on an older version of core-js, the old (buggy) polyfill comes in the bundle and causes this issue in IE (or any other env where native Symbol is not supported yet 😕 )

schmod commented 5 years ago

NPM now prints deprecation warnings for some fairly-common packages due to this issue:

warning @material-ui/core > recompose > fbjs > core-js@1.2.7: core-js@<2.6.5 is no longer maintained. Please, upgrade to core-js@3 or at least to actual version of core-js@2.
schmod commented 5 years ago

A vulnerability has been discovered in all versions of core-js below 3.1.0.

It's unlikely that v1.x will be patched (although 2.x may have enough users that we'll eventually see a patch).

Given the rising popularity of automated vulnerability-scanning systems (and widspread usage of fbjs), a lot of things in the NPM ecosystem are currently being flagged as vulnerable due to fbjs's inclusion of core-js 1.x, which is going to prove to be a moderate nuisance for a lot of folks.

Simek commented 5 years ago

Also installing React Native prints this warning because of included in bundle create-react-class package which supports legacy React classes API and has been excluded from React bundles a while ago.

Not much has been changed in create-react-class package since (it still uses fbjs: "^0.8.9" dependency):

warning react-native > create-react-class > fbjs > core-js@1.2.7: core-js@<2.6.8 is no longer maintained. Please, upgrade to core-js@3 or at least to actual version of core-js@2.

I was trying to find create-react-class repo, but I have failed. NPM link points to React repo, which no longer contains this package in source. GitHub search also did not yield any satisfying results so I have decided to join the discussion here.

asieraduriz commented 4 years ago

This is blocking my create-react-app development as well, in which fbjs requires a core-js version well below any other babel package that requires core-js.
My package-lock (cropped)

"babel-runtime": {
      "version": "6.26.0",
      "resolved": "https://registry.npmjs.org/babel-runtime/-/babel-runtime-6.26.0.tgz",
      "integrity": "sha1-llxwWGaOgrVde/4E/yM3vItWR/4=",
      "requires": {
        "core-js": "^2.4.0",
        "regenerator-runtime": "^0.11.0"
      },
      "dependencies": {
        "core-js": {
          "version": "2.6.10",
          "resolved": "https://registry.npmjs.org/core-js/-/core-js-2.6.10.tgz",
          "integrity": "sha512-I39t74+4t+zau64EN1fE5v2W31Adtc/REhzWN+gWRRXg6WH5qAsZm62DHpQ1+Yhe4047T55jvzz7MUqF/dBBlA=="
        },
        "regenerator-runtime": {
          "version": "0.11.1",
          "resolved": "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.11.1.tgz",
          "integrity": "sha512-MguG95oij0fC3QV3URf4V2SDYGJhJnJGqvIIgdECeODCT98wSWDAJ94SSuVpYQUoTcGUIL6L4yNB7j1DFFHSBg=="
        }
      }
    },
"fbjs": {
      "version": "0.8.17",
      "resolved": "https://registry.npmjs.org/fbjs/-/fbjs-0.8.17.tgz",
      "integrity": "sha1-xNWY6taUkRJlPWWIsBpc3Nn5D90=",
      "optional": true,
      "requires": {
        "core-js": "^1.0.0",
        "isomorphic-fetch": "^2.1.1",
        "loose-envify": "^1.0.0",
        "object-assign": "^4.1.0",
        "promise": "^7.1.1",
        "setimmediate": "^1.0.5",
        "ua-parser-js": "^0.7.18"
      },
      "dependencies": {
        "core-js": {
          "version": "1.2.7",
          "resolved": "https://registry.npmjs.org/core-js/-/core-js-1.2.7.tgz",
          "integrity": "sha1-ZSKUwUZR2yj6k70tX/KYOk8IxjY=",
          "optional": true
        }
      }
    }

When is the core-js upgrade expected to take place?

zpao commented 4 years ago

What packages are people using that still use fbjs < 1.0.0? If it's an fb package we can probably upgrade. If it's something else in the ecosystem, not much we can do. As mentioned, breaking 0.8.x is not an option.