react-native-community / releases

React Native releases
https://github.com/facebook/react-native/
1.5k stars 405 forks source link

0.57.0 Discussion #34

Closed grabbou closed 5 years ago

grabbou commented 6 years ago

Conversation on this thread are limited to 0.57 releases major issues and backport (cherry-pick) requests from commits that are already on master.

An example of a good such request is a bug fix for a serious issue that has been merged into master but did not make the 0.57 RC cut.

In other words, if you cannot point to a particular commit on master, then your request likely belongs as a new issue in http://github.com/facebook/react-native/issues.

vshab commented 6 years ago

It would be nice to see https://github.com/facebook/react-native/commit/fd744dd56ca183933a67e8398e1d20da14a31aab in this release. https://github.com/facebook/react-native/issues/20155 is quite annoying currently.

Thanks.

grabbou commented 6 years ago

Cherry-picked, waiting for CI and will release rc4

Salakar commented 6 years ago

https://github.com/facebook/react-native/pull/20854 - has been synced with master and all CI stages passing now πŸ‘Œ

Titozzz commented 6 years ago

As the RC4 deploy failed due to

capture d ecran 2018-08-31 a 18 03 24

Maybe we can cherry pick https://github.com/facebook/react-native/pull/20854 and release the RC4 with, hopefully all the commits we will need for React Native 0.57? Also, is there anything we can do to fix this step on Circle CI, or is this facebook internal?

grabbou commented 6 years ago

I haven't actually tagged rc.4 yet, so this step was expected to fail anyway. This failure seems very suspicious and should be fixed (CC: @hramos how you were doing that previously?)

Going to cut it now and see where it goes.

Salakar commented 6 years ago

@Titozzz running ssh-keyscan github.com >> ~/.ssh/known_hosts might work to resolve that issue - whether that's needed as part of the script I'm not sure.

Something like:

exec('ssh-keyscan github.com >> ~/.ssh/known_hosts', {silent: true});

before line 65 of the publish-npm.js script maybe.


@grabbou Re: what would be nice to see in the 0.57 release;

Android is missing native Promise reject with a userInfo WritableMap support and also nativeStack support (closes TODO(8850038)).

Have pushed up #20940 for this. Am aware the requirements are master only but this would have been PR'd 1-2 weeks ago if I wasn't in the middle of sorting https://github.com/facebook/react-native/pull/20854 first to get master passing on CI again (it also includes Babel 7 + Metro upgrades + fixing broken CI stages).

christianbach commented 6 years ago

Hi! It would be cool to see facebook/react-native@1cc29c5 in the next RC cut.

ChrisEdson commented 6 years ago

RC4 seems to be tagged - but no release on NPM?

grabbou commented 6 years ago

We are having issues with CircleCI not building correctly. Facebook team will investigate this case in the meantime, going to leave the tag for now. Thread to be updated once we fix the issue.

On Mon, 3 Sep 2018 at 18:36 Chris Edson notifications@github.com wrote:

RC4 seems to be tagged - but no release on NPM?

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/react-native-community/react-native-releases/issues/34#issuecomment-418158277, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWcxol1WB1u2JZryqnPKZVr0a49JoHJks5uXVqGgaJpZM4VCx2F .

Salakar commented 6 years ago

@grabbou https://github.com/facebook/react-native/pull/20854 - addressed all CI failures on master - or are these other CI issues? (it's also approved for importing already)

All CI stages are passing on that PR - the last failure was the Friday/Saturday NPM outage (which was also likely the cause of the import failure)

Titozzz commented 6 years ago

They are having issues with the new NDK, @Salakar if you want to follow what is happening on a branch I use this syntax https://circleci.com/gh/facebook/react-native/tree/<branch_name>, for example https://circleci.com/gh/facebook/react-native/tree/0.57-stable

birkir commented 6 years ago

@Titozzz It's been stale for two days actually.

sercand commented 6 years ago

It seems react-native docker build image is circleci/android:api-27-node8-alpha but there is also circleci/android:api-27-ndk-r17b at docker hub. api-27-node8-alpha don't have NDK in it but api-27-ndk-r17b don't have node and yarn. Therefore react-native can't use both of them.

birkir commented 6 years ago

There is an circleci task to install NDK on the node8 image

edit: Maybe the install ndk steps have to be added to the publish step:

  publish:
    <<: *android_defaults
    steps:
      - checkout

+      - run: *create-ndk-directory
+      - restore-cache: *restore-cache-ndk
+      - run: *install-ndk
+      - save-cache: *save-cache-ndk

      - restore-cache: *restore-yarn-cache
      - run: *yarn
sercand commented 6 years ago

@birkir You are correct it has Install Android NDK setup on test_android step but not in the publish step. It is removed at this commit.

hramos commented 6 years ago

I was out sick the past week, so I missed all of these updates. I'm re-tagging 0.57.0-rc.4 and having it go through Circle again.

birkir commented 6 years ago

Thanks @hramos - It went through, 0.57.0-rc.4 is out

hramos commented 6 years ago

https://github.com/facebook/react-native/commit/1cc29c5 has been cherry-picked, but didn't make it into rc.4.

fungilation commented 6 years ago

Any difference in upgrade procedure, coming from rc1? Upgrading with react-native-git-upgrade, do we still need this in package.json dependencies: "metro-react-native-babel-preset": "0.43.5", and

  "resolutions": {
    "@babel/core": "7.0.0-beta.56",
    "babel-core": "7.0.0-bridge.0"
  }

?

kelset commented 6 years ago

Thanks for taking care of that @hramos!

@fungilation I think you should only need the bridge version, and you should check the package.json for making sure which version of Metro is the latest on branch.

When we will release 0.57.0 (which is, hopefully, soon, based on the feedback on rc4) we'll make sure to include a list of manual steps to upgrade your apps.

simonwang6666 commented 6 years ago

@kelset Will it possible to replace global babelHelper and @babel-external-helper to @babel/runtime for better compatibility to third-party library since RN0.56.0+ is using babel7, some helpers such as applyDecoratedDescriptor is not defined in global babelHelper

kelset commented 6 years ago

@wangshangming could you open a PR for that? I think that it would be easier to visualise and understand how it affects the codebase.

That said unless it fixes a precise bug that is a release-blocker I don't think it will make it for 0.57.0. But maybe for a future minor release.

janhesters commented 6 years ago

This might be a nooby question, but how can I find the git repository for rc4? 😊

Aidurber commented 6 years ago

@janhesters Just the tag in the react-native repo: https://github.com/facebook/react-native/tree/v0.57.0-rc.4

newyankeecodeshop commented 6 years ago

@wangshangming @kelset There is a PR open in metro to replace babel helpers with the babel runtime. Hopefully it will be included in the next major release of metro? https://github.com/facebook/metro/pull/198

simonwang6666 commented 6 years ago

@newyankeecodeshop yeah that's a way to solve the issue but still a better way to remove the babelHelper/regeneratorRuntime since it can not always catch up with the latest babel-helper

kelset commented 6 years ago

thanks for the link! Anyway let's keep this conv about existing commits and/or issues potentially blocking the release - again, @wangshangming, a PR would be the best way to discuss it :)

Salakar commented 6 years ago

Anyone else getting duplicate module issues on rc4? Looks like metro-visualizer installed as part of react-native causing it:

EDIT: resolved, this was an invalid providesModuleNodeModules param on metro.config.js causing the issue - thanks @Titozzz for helping me out.

(node:91698) UnhandledPromiseRejectionWarning: 

Error: jest-haste-map: @providesModule naming collision:
  Duplicate module name: react

  Paths: 

    <PROJ_ROOT>/node_modules/react/package.json 
        collides with 
    <PROJ_ROOT>/node_modules/rc-hammerjs/node_modules/react/package.json

This error is caused by a @providesModule declaration with the same name across two different files.
    at setModule (<PROJ_ROOT>/node_modules/jest-haste-map/build/index.js:462:17)
    at workerReply (<PROJ_ROOT>/node_modules/jest-haste-map/build/index.js:512:9)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

yarn why rc-hammerjs outputs:

image

Should react-native be more explicit as to what metro packages it requires rather than blanket requiring all of metro which adds all the metro packages (package.json)?

yarn why metro-visualizer:

image

Quite a lot of unnecessary packages?

janhesters commented 6 years ago

Will there be a fix for animations on Android? This issue got closed even though it was unresolved. I'm asking because I'm currently facing issues with animations on Android in the app that I'm building.

Titozzz commented 6 years ago

@salakar I've updated my app to 0.57.0-rc.4 from 0.56 with nearly no issues. The only duplicate I had is due to the fact that I have multiple react-native In my workspace yet I manage to deal with the blacklistRE param in the metro.config.js.

I'd like to make sure that 0.57 release is as smooth as possible for everyone, can you DM me on twitter so that we can find out what's wrong with your setup without polluting this thread too much πŸ˜… ?

fungilation commented 6 years ago

Happy to report that upgrading to rc4 caused minimal fuss. I could even take out metro-react-native-babel-preset dep and babel-core resolutions in my package.json, I've verified in my yarn.lock that metro correctly resolves to 0.45.1 and babel* resolves to 7.0.0 currently.

Minimal fuss except https://github.com/facebook/react-native/issues/21020. I suggest a one liner change in gradle-wrapper.properties before 0.57 release.

Salakar commented 6 years ago

@Titozzz have DM'd you - thanks πŸ‘

EDIT: resolved, this was an invalid providesModuleNodeModules param on metro.config.js causing the issue - thanks @Titozzz for helping me out.

yurykorzun commented 6 years ago

Why babel 6 is installed along with babel 7? I have an issue with "Unexpected token '...'. Expected a property name." in one of my projects after updating to 0.57 rc4. Can't figure out it if it's because of the update or I have some missing configuration.

yarn why babel-core

[1/4] Why do we have the module "babel-core"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "babel-core@6.26.3"
info Reasons this module exists
   - "react-native#metro" depends on it
   - Hoisted from "react-native#metro#babel-core"
   - Hoisted from "react-native#fbjs-scripts#babel-core"
   - Hoisted from "jest#jest-cli#jest-runtime#babel-core"
   - Hoisted from "jest#jest-cli#jest-config#babel-core"
   - Hoisted from "react-native#metro#babel-register#babel-core"
   - Hoisted from "react-native#metro#metro-visualizer#emotion#babel-plugin-emotion#babel-core"
fungilation commented 6 years ago

@yurykorzun that's interesting. I've upgraded (by react-native-git-upgrade) to rc4, and

@babel/core in my yarn.lock shows 7.0.0, but babel-core shows multiple 6.x deps that resolves to 6.26.3.

I can run my app in android emulator fine on Windows 10 with this though. What's the difference between babel/core and babel-core anyway?

I searched through RN's github repo, and the only reference to "babel-core" (not babel/core) is in https://github.com/facebook/react-native/blob/master/react-native-git-upgrade/package.json, which specifies ^6.18.0.

@kelset, is this correct or should it be version bumped (for git-upgrade and manual upgrade)? To "babel-core": "7.0.0-bridge.0" or just "babel-core": "7.0.0"?

LinusU commented 6 years ago

@fungilation

I searched through RN's github repo, [...]

I would recommend looking in the Metro repository, I think that could be where it's coming from.

fungilation commented 6 years ago

The bigger question before even where it's from, is what is the intended/correct version(s) post-install for babel/core, babel-core.

LinusU commented 6 years ago

The correct version for React Native 0.57 should be 7.x.x. As of rc.4 we upgraded to the stable Babel 7.

Titozzz commented 6 years ago

I think what is really important is that your build is done with Babel 7 and that Babel 6 does not conflict with that. If then we still have a dependency such as a cli that require babel-core or babel-runtime to run, that's fine and it can get bumped later if needed.

birkir commented 6 years ago

https://github.com/ueno-llc/react-native-starter

Fully working with typescript and everything, literally only thing I had to do was bump the version and modify the metro config a bit.

Try removing the npm/yarn lockfile and install again with just babel 7 (no bridge)

kelset commented 6 years ago

@fungilation "babel-core" is the old implementation, the new one is "@babel/core". The reason why we suggest to have "babel-core": "7.0.0-bridge.0" it's because it's a special version that should help you handle scenarios in which you may have old packages that still use babel 6.

That said, again, pls keep this issue solely on the topic of the release and eventual commits to cherry pick.

matei-radu commented 6 years ago

Since we updated Gradle, should we also include facebook/react-native@4dfdec9 so to release 0.57 with all Gradle parts updated?

Then again, it's not a priority since the old compile configuration will be removed only at the end of the year in a future update.

xzilja commented 6 years ago

Hey guys, since 0.57.0-rc.4 added support for "out of the box typescript" I decided to give it a go and stumbled across following issue:

It seems that it compiles .ts and .tsx files fine, however I have module resolution set up in my repo and am getting following error:

Loading dependency graph, done. error: bundling failed: Error: Unable to resolve module @myApp/services/validation from /Users/me/Documents/Repositories/myApp/packages/myApp/src/store/AuthStore.ts: Module @myApp/services/validation does not exist in the Haste module map

This used to work with typescript setup pre 0.57.x. Am I wrong in assuming that rn will respect existing tsconfig.json files as default or is additional configuration in babel needed for this?

yurykorzun commented 6 years ago

I decided to try to narrow down the problem I was having with 0.57 rc4 and try to find where the exception Unexpected token '...' was coming from. After adding react-navigation to the empty project I get the same error right away. It looks like react-navigation cannot resolve the object rest spread operator plugin. It works with RN 0.54

This is probably off topic and I will post it to react-navigation issues.

grabbou commented 6 years ago

@matt-block yes, we should. Unfortunately, it raises one important concern that I have - I described it in the commit. Let's look at it and discuss it for a moment before we decide to cherry-pick it. I am afraid it might be deal breaking for existing users of React Native link.

That said, I could potentially spend some time tomorrow getting this fixed, as we have to ship it (who wants stable release with warnings), yet we need this one fixed and fully supportive of both formats, at least for one release.

ikesyo commented 6 years ago

https://github.com/facebook/react-native/commit/0bf56e0746c1e60204996558b15ba425b12ed457 should be cherry-picked to make the new WKWebView backend usable (the feature is listed in the 0.57 changelog's Highlights section #40).

rowinbot commented 6 years ago

Can we get facebook/react-native@2307ea6 cherry-picked into next release?

Actually, we're unable to mask an input or append a validation 'cuz TextInput is not taking the value being passed as a prop as the actual input value.

hramos commented 6 years ago

We should cherry pick @ikesyo’s suggested WKWebView commit and promote to stable. I’d leave the TextInput commit for the next release. We have to cut at some point; after all, we should really be only cherry picking commits for fixes that remained incomplete (as in, the release was cut when some commits related to the fix had not landed yet), as well as fixes for any major issues that arises during the RC process.

jonathanglasmeyer commented 6 years ago

Just tracked down an issue while trying to get a running project with RC4: the react-native local-cli fails with

ReferenceError: regeneratorRuntime is not defined
    at getCliConfig (/Users/jwerner/projects/rn-test/Foo/node_modules/react-native/local-cli/core/index.js:128:44)

when babel is setup with babel.config.js:

// babel.config.js
module.exports = {
  "presets": ["module:metro-react-native-babel-preset"]
}

Everything works when the same babel configuration is applied in the old .babelrc format.

I traced down the problem by inserting console.logs in node_modules/@babel/register/lib/node.js, function compile. When using babel.config.js you can see that all plugins of module:metro-react-native-babel-preset are being applied to the node_modules/react-native/local-cli/core/index.js file; when using .babelrc that's not the case. The code breaks because apparently regeneratorRuntime call is inserted, but babel/polyfill (or regenerator-runtime/runtime) import is not happening.

Repository with working state With a broken state

Is this behaviour known? How are users expected to make use of babel.config.js?

kelset commented 6 years ago

@jonathanglasmeyer open a dedicated issue for that.

I agree with @hramos statement, we want to get to 0.57.0 now without too many new commits.

Moreover, I haven't seen any major "release-blocker" issues so I think we may want to proceed to 0.57.0 soon.

michaelknoch commented 6 years ago

I tried to run my app with rn 0.57rc 4 but it seems that weird things are happening when transforming local-cli/server/server.js. I migrated from babel-preset-react-native to metro-react-native-babel-preset

> node node_modules/react-native/local-cli/cli.js start

/Users/michaelknoch/dev/flow/smartphone-app/node_modules/@babel/core/lib/transformation/normalize-file.js:209
    throw err;
    ^

SyntaxError: /Users/michaelknoch/dev/flow/smartphone-app/node_modules/react-native/local-cli/server/server.js: Unexpected token, expected ";" (9:32)

   7 |         })
   8 |       );
>  9 | /* remotedev-server end */ code is licensed under the MIT license found in the
     |                                 ^
  10 |  * LICENSE file in the root directory of this source tree.
  11 |  *
  12 |  * @format