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

chore: bump metro to 0.53 #222

Merged
merged 1 commit into from
Mar 15, 2019

Conversation

belemaire
Copy link
Contributor

Summary:

Bump metro dependencies from 0.51 to 0.53, mostly to propagate some recent bug fixes.
This version bump does not seem to have any impact on the CLI.

In terms of metro breaking changes, in 0.52.0, the dependencies command was renamed to get-dependencies. This change already seems to be accounted for, in PR #100.
In 0.53.0, the only breaking change is related to the use of custom (non production) BABEL_ENV. This is unusual, so it shouldn't impact most users, and workaround is documented in the PR in case some users are facing the problem.

Test Plan:

Close #210

@thymikee
Copy link
Member

As you can see #100 is marked onhold for 2.0 release, because it's a breaking change (even though RN stable is not yet using the new CLI).

If we don't want to make this PR breaking, can you proxy dependencies to get-dependencies?

What do you think @grabbou, should we cut 2.0 already or break 0.59 RCs (and maybe some other consumers?) just a bit more?

@grabbou
Copy link
Member

grabbou commented Mar 12, 2019

Thanks for sending this over! React Native still uses metro-react-native-babel-transformer that is 0.52.0, but I don't think we need to keep the versions in sync.

(I've checked that package source code and it doesn't have a peer dependency nor dependency on any of the Metro packages that we use here).

CC: @cpojer - shall we keep Metro version here in sync with https://github.com/facebook/react-native/blob/master/package.json#L95-L96 or we are good to just keep updating it? (since you follow master anyway)

@cpojer
Copy link
Member

cpojer commented Mar 12, 2019

Yes we should keep it in sync in both places.

@grabbou
Copy link
Member

grabbou commented Mar 12, 2019

Then we need to figure out an update strategy to avoid issues in the future. Right now, any major/minor update to the CLI will be automatically pulled by all React Native installations that use it (from 0.59 onwards).

Shall we scope React Native CLI version for every React Native release to patches only?

@thymikee thymikee added this to the 2.0 milestone Mar 12, 2019
@grabbou
Copy link
Member

grabbou commented Mar 12, 2019

Ideally, Metro would be a React Native dependency so it could be dynamically installed based on the version needed.

@cpojer - checking the packages that React Native depends on - they don't seem to have any Metro related dependencies. Do they really depend on other Metro packages?

@cpojer
Copy link
Member

cpojer commented Mar 12, 2019

@grabbou the CLI is responsible for all the interactions with Metro so when we extracted the CLI, we were able to remove the direct dependency between RN and Metro.

For the metro-react-native-babel-transformer, maybe we can set it to * and set it to the latest that the CLI supports upon RN releases.

@grabbou
Copy link
Member

grabbou commented Mar 12, 2019

That would make sense and let us ship updates to React Native a bit easier and faster. Is this something we can try doing for 0.60 release?

@belemaire
Copy link
Contributor Author

Thanks for the update.

We're good on our end with RN 0.59 that was just released with metro 0.51 (via current 1.x line of he CLI). As part of our automation, we're manually patching metro with the fix we need from metro 0.52, so that we have a workaround for now (for #210).

@thymikee do you still want to proxy dependencies command to metro get-dependencies command ? I'm a bit confused here because PR #100 completely removes the dependencies command from the CLI and is planned to be merged for 2.0 along with my PR, and it seems like there was no plan to replace it with a proxy (as far as I understand the PR comments).

@thymikee
Copy link
Member

To be frank, I recently noticed that react-native dependencies is actually broken on CLI 1.x. Not sure how many projects use that, but I assume not too many.

By proxying (and actually fixing) dependencies to get-dependencies we could squeeze in Metro 0.53 to 1.x without a breaking change, which would be nice, isn't it?

@belemaire
Copy link
Contributor Author

belemaire commented Mar 12, 2019

But per comments from @grabbou and @cpojer it does not seem possible to bump metro version in CLI 1.x because of the metro-react-native-babel-transformer dependency in RN 0.59 that is still set to 0.51.0 and metro dependencies need to match (do they really ? the CLI is currently using version range ^0.51.0 which resolves to 0.51.1 whereas RN 59 is using fixed version 0.51.0 so they are not at this point using the exact same versions (same minor only)).
So if the version mismatch restriction still applies, there is no way to squeeze metro 0.53 to 1.x anyway.

@thymikee
Copy link
Member

Oh, I thought they're updated already. Then don't bother and just wait for us to merge it in a couple of days :). Sorry for misunderstanding!

@cpojer
Copy link
Member

cpojer commented Mar 15, 2019

Could we change this PR to update Metro to 0.53.1?

It seems like we can merge this afterwards, right @thymikee?

@thymikee
Copy link
Member

Yup, we're gonna need to release 2.0 from that point.

@cpojer
Copy link
Member

cpojer commented Mar 15, 2019

I changed it to 0.53.1, merging now :)

@cpojer cpojer merged commit 0c6a74d into react-native-community:master Mar 15, 2019
@Vyazovoy
Copy link

Vyazovoy commented Jun 7, 2019

Hi, @cpojer ! Is it possible to update Metro to 0.54.1 in 1.x That Metro version has a fix for custom BABEL_ENV

@thymikee
Copy link
Member

thymikee commented Jun 7, 2019

@Vyazovoy I just merged a PR with that, will be available in RN 0.60.0-rc.1 :)

@thymikee
Copy link
Member

thymikee commented Jun 7, 2019

But I don't think we can put this in 1.x because there are breaking changes

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.

Bump metro dependencies
5 participants