facebook/react-native

Official, gradual path to RN packages not publishing untranspiled code

ljharb opened this issue ยท 28 comments

The current practice in the RN community for package authors is to publish untranspiled code, to npmignore babelrc, and for the RNP to transpile node_modules.

While this may work great for the RN community, this defies many best practices in the wider node/npm/JS community:

  • package.json's "main" only points to ES5 commonJS
  • npm explore foo && npm install && npm test should work (note this isn't universal, but it's frustrating to be asked to npmignore .babelrc and break this use case due to the RNP)

In addition, this practice kind of permanently cements RN packages with the transpiler settings that the RNP uses - which, since it includes not-yet-stage-4 features, are quite likely to change in a breaking way in the future.

Here's my proposal to enable the benefits of the current approach (cleaner stack traces, optimization opportunities within the RNP, etc) in a way that will allow RN packages to rejoin the wider JS ecosystem:

  1. Designate a new top-level package.json key - let's go with "sources" for the time being. This contains an object. All subkeys of "sources" are optional.
  2. Within "sources":
  3. "raw" points to the entry point for utterly untranspiled code. .babelrc (or other babel config locations), if provided, configure how this code should be transformed.
  4. "react-native-v1" (swap out v1 for any versioning scheme; this frees up the RNP to change their preferred transpiler settings freely.
  5. "node4", "node5", "node6", "node7", etc: code transpiled down to features matching the indicated major node version, following https://www.npmjs.com/package/babel-preset-node7, https://www.npmjs.com/package/babel-preset-node6, etc.
  6. "es2015", "es2016", etc: code transpiled down to features that are included in the given year of publication of the ecmascript standard.
  7. The RNP ships with support for this, with priority: ["react-native-", "node-" (>= the major node version, obv), "main" (with transpiling)]. This change is loudly communicated with blogs, tweets, and documentation.
  8. At some future point (this can be as soon, or as indefinite, as is deemed necessary), the RNP will cease transpiling "main" - since the other "sources" don't require looking for babel configs, this will be when RN packages can (and should) publish their .babelrcs again.

This approach allows for supporting other languages, future transpiler settings, etc, in a nonbreaking way, without infecting the npm ecosystem with untranspiled "main" entry points (inhibiting people's ability to share RN packages across normal react). A package will always be able to choose to omit "main" entirely, which will clearly and explicitly convey to users that their package must not be used without the RNP.

Further reading (credit @ide): https://gist.github.com/ide/e7b9181984933ebb0755c7367a32e7e8

I'm fully in support with stopping to transpile node modules. I have to consider a bit more deeply what the actual solution will look like and how many sources on average will be published. We shouldn't encourage people to publish their package with ten sets of differently compiled code.

For RNP, I would recommend the following:

  • At some point in the future, start warning for every node_module that it has to compile. Communicate with the community what we are planning to do and encourage package developers to compile their code. Provide an easy workflow to make this happen.
  • At some point later, we should stop compiling node_modules by default. If something breaks, we'll encourage users to manually whitelist the package that fails.

There is no reason not to enable the escape hatch; especially since there is no point in breaking existing users completely in a way they can't move forward.

ide commented

@cpojer I think it would be worse if the packager didn't transpile node_modules at all. We get a lot of benefits because the packager is able to transpile node_modules that are mentioned in the gist. I would be on board with your RNP proposal if this bullet point came before the other two:

  • If a package specifies sources.react-native-vX, transpile the source code from that entry point. Skip the other steps since this package author has explicitly said they want RNP to transpile the code, we don't need to print a warning.

We shouldn't encourage people to publish their package with ten sets of differently compiled code.

Ten might be excessive, for the sake of this conversation wrt React Native why don't we focus on just two? Package authors will choose which sources they want to publish. If someone is authoring react-native-widget they will likely just want to publish main and sources.react-native-v1. A package like lodash might publish just main and es2016. I think the package tarballs will compress quite well too.

I'm not sure i see the harm in publishing packages with multiple sets of differently compiled code, a great many packages already publish two. That said, we don't need to encourage that - it'd be more that we were encouraging them to select one thing, and publish 1) ES5, 2) their choice, and 3) very optionally the "raw". The goal is to enable/encourage publishing ES5, and explicitly informing the RNP of which transpiler settings it should be using.

skevy commented

Somewhat related -- I just made this: https://github.com/skevy/babel-preset-react-native-syntax

This preset would help React Native package authors transpile their code down to what is described here as "sources.react-native-v1".

(Also is useful today, so that people in the RN community who want to use decorators, stage-0 or some stage-1 features in their packages don't have to require their users to use some other babel-preset (like babel-preset-react-native-stage-0), and also can avoid transpiling all the way down to ES5).

Is there a guide for publish RN components to npm?

Closing this issue because it has been inactive for a while. If you think it should still be opened let us know why.

@hramos it should absolutely still be open for all the same reasons; RN packages are polluting the ecosystem, and this issue proposes a path to stop that. Please reopen.

Has there been any movement here? I don't disagree on the usefulness of the work, rather it seems like the issue is not actively being worked on.

Unfortunately it doesn't seem that way; but issues should be closed when they're resolved or wontfixed, not just because nothing has happened yet.

Unfortunately we have over 1,000 issues open. Closing inactive issues will allow us to focus on what is actively being worked on. Closing the issue won't delete the conversation, and we can easily re-open when things get moving.

@cpojer any thoughts on starting this process? The sooner it's begun the more time we have to gradually migrate module authors over.

skevy commented

@ljharb hey jordan! The packager is actually getting moved out of this repo pretty soon, into it's own thing (which should be easier to iterate on, discuss, take PRs, etc). I'll try to remember to open this issue there (unless you beat me to it). This should be happening in a couple weeks. :)

Thanks, that sounds great!

In the process of closing stale issues, I missed that @ljharb was the original author of this issue. I think it's fair to re-open an issue where the OP is actively interested.

I'll re-open this for now, so we don't lose track of it when the packager is moved out.

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

  • Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
  • If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
  • Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

@hramos any chance this could still be left open? I'm still actively interested.

Of course. Re-opening. This issue shouldn't be closed again now that it has the Discussion label. Sorry about that.

stale commented

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

stalebot: this issue has the For Discussion label.

No supper for stalebot tonight.

stale commented

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

This is definitely still forever going to be a problem until the RN team fixes it.

@cpojer what should we do here? This issue seems like a better fit for the discussions-and-proposals repo, which didn't exist at the time the issue was created.

(Stale bot is not supposed to nag on Discussion issues, but I recently renamed the "Type: Discussion" label but neglected to update Stale bot's config. Sorry about that)

Don't currently have any plans to change what we are doing in RN.

@cpojer can you elaborate on whether and why you've changed your mind since your comment here? Or are you just saying it's not roadmapped right now.

I haven't changed my mind, but I doubt anybody is going to change things and I won't have time to spend on this either.

I'm gonna close this now as we aren't likely to make any progress on this in the next six months, and this issue has been open for a very long time without any progress. If anybody wants to volunteer to change the status quo, please create an issue with a thorough plan in our discussions-and-proposals so that we can agree and that you can start executing on this.