Skip to content

[act] Wrap IsThisRendererActing in DEV check#16259

Merged
acdlite merged 2 commits into
react:masterfrom
acdlite:wrap-act-check-in-dev
Jul 31, 2019
Merged

[act] Wrap IsThisRendererActing in DEV check#16259
acdlite merged 2 commits into
react:masterfrom
acdlite:wrap-act-check-in-dev

Conversation

@acdlite

@acdlite acdlite commented Jul 31, 2019

Copy link
Copy Markdown
Collaborator

So that it doesn't leak into the production bundle. Follow-up to #16240.

So that it doesn't leak into the production bundle. Follow-up to react#16240.
@sizebot

sizebot commented Jul 31, 2019

Copy link
Copy Markdown

No significant bundle size changes to report.

Generated by 🚫 dangerJS

@acdlite

acdlite commented Jul 31, 2019

Copy link
Copy Markdown
Collaborator Author

@threepointone Just to confirm, this isn't supposed to affect prod, right? There are some tests that are failing (correction: one test) but I assume that was just an oversight in the last PR.

@acdlite

acdlite commented Jul 31, 2019

Copy link
Copy Markdown
Collaborator Author

I'm going to assume that this doesn't affect prod and we can follow up later if that's not the case. All the act stuff should be dev-only, or alternatively, in a separate testing build.

@threepointone

Copy link
Copy Markdown
Contributor

We use this sigil not just for warnings now, but also for deciding when to flush, so it does actually affect prod?

@acdlite

acdlite commented Jul 31, 2019

Copy link
Copy Markdown
Collaborator Author

Is this the only case? We really should not be leaking act behavior into the main production bundles.

@acdlite

acdlite commented Jul 31, 2019

Copy link
Copy Markdown
Collaborator Author

What is the use case for running act tests in production?

@threepointone

Copy link
Copy Markdown
Contributor

I think the discussion we had was people might be running tests with prod builds? iirc. Happy to not support it if so, or move to making testing builds.

@acdlite

acdlite commented Jul 31, 2019

Copy link
Copy Markdown
Collaborator Author

Since this only affects Concurrent and Sync mode I'll merge this and we can pick up discussion elsewhere.

@acdlite acdlite merged commit f939df4 into react:master Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants