Skip to content

Reduce unnecessary warning while including JSX transformer in browser#2149

Merged
zpao merged 1 commit intofacebook:masterfrom
imcotton:jsx-reduce-warning
Sep 23, 2014
Merged

Reduce unnecessary warning while including JSX transformer in browser#2149
zpao merged 1 commit intofacebook:masterfrom
imcotton:jsx-reduce-warning

Conversation

@imcotton
Copy link
Copy Markdown
Contributor

@imcotton imcotton commented Sep 5, 2014

Yet prevents calling loadScripts with empty array, hope it helps.

@zpao
Copy link
Copy Markdown
Member

zpao commented Sep 5, 2014

I kind of want to keep it so it's noisy and bothers you and you don't forget to remove it in production. If it's not processing scripts and being quiet it becomes easier to leave it in in production. That might not really be our place to tell you, but could be helpful.

I don't feel super strongly though, so would be interested in hearing what you think.

@sophiebits
Copy link
Copy Markdown
Collaborator

I'm inclined to agree here and leave it as is.

@imcotton
Copy link
Copy Markdown
Contributor Author

imcotton commented Sep 6, 2014

I agree having it for production noticing would be handy for sure, but on the other hand, personally I feel like maybe too aggressive while you only have it included but not encounter one script with text/jsx type, for instance to access exposed api exec or transform.

I have been using the same on-the-fly feature from CoffeeScript and Less in a lot of side projects myself, which only need to served by a static http server e.g. GitHub Pages, yet not have to store compiled results into repository which, a) reduce redundancy b) does not depend on any command line compiler.

Not to using in production is the best practice indeed, but sometime in reality it’s not all about production concern. Seems a little bit OT, so here is one approach in jsFiddle I have managed to combine CoffeeScript with JSX transformer, which leverage the power from both side.

@zpao
Copy link
Copy Markdown
Member

zpao commented Sep 7, 2014

That's a good point. I still want to be noisy so this seems like a fine compromise. You fine with that @spicyj?

I was trying to think of other compromises (leading idea I had was starting a 5 second timeout and tracking if any other APIs were called, then warning if not) but they're all a bit overkill.

@imcotton
Copy link
Copy Markdown
Contributor Author

imcotton commented Sep 7, 2014

@zpao You remind me that in Less, it separates logLevel from development and production environments, might be another approach I think.

@zpao
Copy link
Copy Markdown
Member

zpao commented Sep 7, 2014

@imcotton Oh that's interesting. We haven't really done anything with log levels (but we've talked about it). Definitely might be worth exploring in JSXTransformer first before we do anything in React...

zpao added a commit that referenced this pull request Sep 23, 2014
Reduce unnecessary warning while including JSX transformer in browser
@zpao zpao merged commit 7339f8b into facebook:master Sep 23, 2014
@zpao
Copy link
Copy Markdown
Member

zpao commented Sep 23, 2014

Let's just do it and if we need to change out minds later and/or refine this, so be it.

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.

3 participants