-
Notifications
You must be signed in to change notification settings - Fork 17
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
Question about compilation. #1
Comments
Hi @jordow and thanks for the feedback! This require.js plugin should definitely notify the user not to use in-browser-JXS-transformation in production. Do you have a better idea than just writing this message to the browsers JS console? |
Browser console is a good start. |
I've been able to get the compiled jsx in a built version with r,js using PR #5 and a build config that has the following: (note this isn't the full config, just the relevant parts)
What is does is write the compiled version to the file with 'jsx!' removed whenever found. So something like: define('Component1', ['jsx!Component2'], function (Component2) {}); becomes: define('Component1', ['Component2'], function (Component2) {}); This isn't the best solution due to the potential to replace instances of 'jsx!' where it isn't desirable, but it is at least working. |
@camspiers Can you elaborate on your build setup? Despite excluding JSXTransformer and jsx, I am getting errors since jsx depends on JSXTransformer. I am using your patch from #5:
|
My
My
And the command I am running is:
Excuse the old versions, the project was a while ago. Hope this helps! |
@ssorallen this error is caused by some preprocessing that removes 'use strict' from In the original file, |
I figured out how to make it work. Used @camspiers config:
And replaced occurrences of |
@philix, that sounds like the bit I was missing. Thanks for pointing that out. I will give that a shot and see if that fixes my build. |
I still couldn't get it working the way described above, but used the ideas to solve it in (what I think is) a different way. I remove "jsx!" and do the JSX->JS transformation myself before handing the files over to r.js and almond. I have my files in ./app, ./app/jsx (this is where the JSX files live) and ./lib. My requirejs config is like this: requirejs.config({ baseUrl: "/js2/lib", paths: { app: "/js2/app", appjsx: "/js2/app/jsx", jsx: "jsx", JSXTransformer: "JSXTransformer" }, shim: { exports: "JSXTransformer" } }); and my build.js file is like this: ({ baseUrl: "../build", out: "main-built.js", name: "lib/almond", include: ["app/a"], exclude: [], preserveLicenseComments: false, generateSourceMaps: true, optimize: "uglify2", insertRequire: ["app/a"], onBuildWrite: function(moduleName, path, singleContents) { return singleContents .replace(/console\.(log|assert)\([^))]*\);/g, ""); }, paths: { appjsx: "jsx" } }) and my Makefile can be found at https://gist.github.com/maglar0/9762505 . |
@maglar0 what are the error messages? r.js fails for all sorts of reasons. If you're getting facebook/react#972 may help too. |
@philix It works when I set sourceMap to false as you did in that link, together with the earlier change of === 'use strict' to === 'use '+'strict'. Thanks for the effort you have put into solving this, I got totally nonsensical error messages like "undefined is not a function" without any reference to line numbers so didn't know where to start at all. However, I strongly prefer not to fork JSXTransformer.js and the source map of the output when doing it this way references nonsense files like "../app/jsx/d!jsx", thus I will continue to do the JSX transform myself and then hand over the files to r.js. |
@maglar0 your approach avoids a lot of headaches 👍 |
This project is very cool. I'll certainly end up using this. Will require.js ensure that the compilation happens on the server as part of the build step? If not, is there some way that we can message to users that compilation in the browser should not be done in production (due to the huge bloat of file size caused by including the compiler and the CPU required to compile)?
The text was updated successfully, but these errors were encountered: