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

RNW setup: only transpile node_modules #933

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@slorber
Copy link

slorber commented Mar 18, 2019

I think the newly added rule should not apply to files outside of node modules, to avoid messing up with another js loader that may already be present

This is for example required for my Gatsby plugin setup to work, because Gatsby already has its own babel loader for app sources:
https://github.com/slorber/gatsby-plugin-react-native-web/blob/master/examples/todos/gatsby-node.js
https://gatsby-rnw-todos.netlify.com/

RNW setup: only transpile not modules
I think the newly added rule should not apply to files outside of node modules, to avoid messing up with another js loader that may already be present

This is for example required for my Gatsby plugin setup to work, because Gatsby already has its own babel loader for app sources:
https://github.com/slorber/gatsby-plugin-react-native-web/blob/master/examples/todos/gatsby-node.js
https://gatsby-rnw-todos.netlify.com/
@callstack-bot

This comment has been minimized.

Copy link

callstack-bot commented Mar 18, 2019

Hey @slorber, thank you for your pull request 🤗. The documentation from this branch can be viewed here. Please remember to update Typescript types if you changed API.

@slorber slorber changed the title RNW setup: only transpile not modules RNW setup: only transpile node_modules Mar 18, 2019

@ferrannp ferrannp requested a review from satya164 Mar 22, 2019

@ferrannp

This comment has been minimized.

Copy link
Member

ferrannp commented Mar 22, 2019

I think we can update docs to say you need this for Gatsby but it cannot be the main example.

@slorber

This comment has been minimized.

Copy link
Author

slorber commented Mar 22, 2019

@ferrannp just wondering can you expand on your thinking?

For me this does not only apply to Gatsby but would be better for any RNW setup:

  • You are pushing a new rule to an array, so you basically expect the user to already handle his own babel config for his own sources (at least it's what I assumed)
  • That "extra rule" should only be responsible for node modules transpilation (which is not common practice in web envs and unlikely to already be in existing user setup unless he's quite familiar with RNW etc...)
  • That "extra rule" should not conflict with existing user rule for transpiling regular user sources (in ./src)

So all of this depends on weither or not you consider the new rule you push an "extra rule" that completes an existing rule, or if you expect to use a single babel rule and merge it with your RNW rule to form only one rule in the end (but in this case I'd argue the use of push is confusing)

@satya164

This comment has been minimized.

Copy link
Member

satya164 commented Mar 22, 2019

Hey @slorber

Thanks for the PR. I think there's a bit of misunderstanding about the docs. In your PR, you have 2 parts:

  1. The first one is for CRA, where we're pushing an extra new rule. And as you said, it should only include the files in node_modules
  2. The second one is for a vanilla configuration, where we assume that you're configuring everything from scratch and don't have another babel loader config.

But I agree it's very not clear regarding that. I think what we should do is, mention that if the user is already compiling their application code with Babel and have an existing babel-loader config, or using a tool such as CRA, Gatsby, Next.js etc., then they need to add this extra include. But if they don't have an existing babel loader config, then they don't need the include.

@slorber

This comment has been minimized.

Copy link
Author

slorber commented Mar 25, 2019

Hi,

In my PR maybe I should not modify the 2nd part and only modify the first part with push then?

Or maybe both parts should use "push". Because maybe your own transpilation config should not "pollute" the user one? I mean he may not be using flow in his own codebase so asking him to add the flow preset for his whole babel loader may be unnecessary (but probably wouldn't harm much)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.