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

tsconfig - enable allowJs, jsx:"react" #3575

Merged
merged 1 commit into from Mar 12, 2018
Merged

tsconfig - enable allowJs, jsx:"react" #3575

merged 1 commit into from Mar 12, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Mar 12, 2018

jsx: "react" is needed when importing a jsx file from typescript, or using tsx
allowJs: true is needed when importing a js/jsx file from typescript

Cc @karelhala, @martinpovolny

Extracted from #3517
Needed by #3228

`jsx: "react"` is needed when importing a jsx file from typescript, or using tsx
`allowJs: true` is needed when importing a js/jsx file from typescript
@himdel
Copy link
Contributor Author

himdel commented Mar 12, 2018

Testing:

--- a/app/javascript/packs/application-common.js
+++ b/app/javascript/packs/application-common.js
@@ -13,3 +13,9 @@ window.MiqReact = Object.assign(window.MiqReact || {}, {
   mount: mount,
   componentRegistry: componentRegistry
 });
+
+import foo from '../foo';
+console.log('foo', foo);
+
+import foo2 from '../foo2';
+console.log('foo2', foo2);
diff --git a/app/javascript/bar.tsx b/app/javascript/bar.tsx
new file mode 100644
index 000000000..f7b79bcc4
--- /dev/null
+++ b/app/javascript/bar.tsx
@@ -0,0 +1,5 @@
+import React from 'react';
+
+const whatever = () => <div></div>;
+
+export default whatever;
diff --git a/app/javascript/bar2.jsx b/app/javascript/bar2.jsx
new file mode 100644
index 000000000..f7b79bcc4
--- /dev/null
+++ b/app/javascript/bar2.jsx
@@ -0,0 +1,5 @@
+import React from 'react';
+
+const whatever = () => <div></div>;
+
+export default whatever;
diff --git a/app/javascript/foo.js b/app/javascript/foo.js
new file mode 100644
index 000000000..874c12a26
--- /dev/null
+++ b/app/javascript/foo.js
@@ -0,0 +1,2 @@
+import bar from './bar';
+export default bar;
diff --git a/app/javascript/foo2.ts b/app/javascript/foo2.ts
new file mode 100644
index 000000000..8eb6531db
--- /dev/null
+++ b/app/javascript/foo2.ts
@@ -0,0 +1,2 @@
+import bar2 from './bar2';
+export default bar2;

With this PR, running webpack should still succeed.

Without allowJs:

ERROR in ./app/javascript/foo2.ts
(1,18): error TS6143: Module './bar2' was resolved to '/home/himdel/manageiq-ui-classic/app/javascript/bar2.jsx', but '--allowJs' is not set.

Without jsx:

ERROR in ./app/javascript/foo2.ts
(1,18): error TS6142: Module './bar2' was resolved to '/home/himdel/manageiq-ui-classic/app/javascript/bar2.jsx', but '--jsx' is not set.

ERROR in ./app/javascript/bar.tsx
(3,24): error TS17004: Cannot use JSX unless the '--jsx' flag is provided.

ERROR in ./app/javascript/bar.tsx
Module parse failed: /home/himdel/manageiq-ui-classic/node_modules/ts-loader/index.js!/home/himdel/manageiq-ui-classic/app/javascript/bar.tsx Unexpected token (2:36)
You may need an appropriate loader to handle this file type.
| import React from 'react';
| var whatever = function () { return <div></div>; };
| export default whatever;
| 
 @ ./app/javascript/foo.js 7:11-27
 @ ./app/javascript/packs/application-common.js

@miq-bot
Copy link
Member

miq-bot commented Mar 12, 2018

Checked commit https://github.com/himdel/manageiq-ui-classic/commit/f5594378eb027421dbd95ed8dec29bcd1bd9ac54 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@martinpovolny martinpovolny merged commit ae9726f into ManageIQ:master Mar 12, 2018
@martinpovolny martinpovolny added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 12, 2018
@martinpovolny martinpovolny self-assigned this Mar 12, 2018
@himdel himdel deleted the tsconfig-js,jsx branch March 12, 2018 15:39
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.

None yet

3 participants