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

Support Rollup input extension .jsx #143

Closed
sisp opened this issue Jun 14, 2019 · 6 comments · Fixed by #474
Closed

Support Rollup input extension .jsx #143

sisp opened this issue Jun 14, 2019 · 6 comments · Fixed by #474
Labels
kind: bug Something isn't working kind: feature New feature or request

Comments

@sisp
Copy link

sisp commented Jun 14, 2019

Current Behavior

The file src/index.jsx is not recognized as a Rollup input:

https://github.com/palmerhq/tsdx/blob/943648edb06cd131f2c1751120cd1301804b480b/src/index.ts#L64-L69

Expected behavior

It should be possible to have the file src/index.jsx as the Rollup input.

Suggested solution(s)

async function jsOrTs(filename: string) {
  const extension = (await isFile(resolveApp(filename + '.ts')))
    ? '.ts'
    : (await isFile(resolveApp(filename + '.tsx')))
    ? '.tsx'
    : (await isFile(resolveApp(filename + '.jsx')))
    ? '.jsx'
    : '.js';

  return resolveApp(`${filename}${extension}`);
}

Your environment

Software Version(s)
TSDX 0.6.1
TypeScript 3.5.1
Browser Chromium
npm/Yarn npm
Operating System Ubuntu 18.04
@sisp
Copy link
Author

sisp commented Jun 14, 2019

Related to the this issue, TSDX currently does not seem to support importing *.js(x) files from *.ts(x) files because rollup-plugin-typescript2 is not configured to process *.js(x) files.

https://github.com/palmerhq/tsdx/blob/943648edb06cd131f2c1751120cd1301804b480b/src/createRollupConfig.ts#L146-L162

The fixed configuration looks like this:

typescript({
  // everything from above, plus ...
  include: ['*.(t|j)s+(|x)', '**/*.(t|j)s+(|x)'] // default is ["*.ts+(|x)", "**/*.ts+(|x)"]
})

@swyxio
Copy link
Collaborator

swyxio commented Jun 16, 2019

tsdx is quite evidently a typescript-first project. not a priority for me to support .js{x} but lets see what jared says

@sisp
Copy link
Author

sisp commented Jun 16, 2019

That's true, but I was thinking to align tsdx's features with CRA and CRA with --typescript set also resolves .js{x} files.

@agilgur5
Copy link
Collaborator

I'm not having this issue, but I think if tsdx supports .js it should also support .jsx. That would also add better support for gradual migrations to TS.

@myklt
Copy link

myklt commented Sep 8, 2019

Isn't the problem here that Typescript doesn't support .jsx as a file extension for files containing jsx? As far as I know only .tsx is supported. More details: microsoft/TypeScript#30503

@agilgur5
Copy link
Collaborator

agilgur5 commented Jan 30, 2020

Wanted to update folks on this issue as this was discussed in #443 as well and I've made a few related PRs to improve JSX support.

Per #443, TSDX actually already supports JSX, you just need to make sure your tsconfig.json has compilerOptions.allowJs set to true and that your include picks it up. TSDX does not by default allow JS or JSX but does support them when they are configured.

Isn't the problem here that Typescript doesn't support .jsx as a file extension for files containing jsx? As far as I know only .tsx is supported. More details: microsoft/TypeScript#30503

I'm not sure if this is the case or is still the case, but it works in TSDX. That might only be because TSDX runs Babel on top of rollup-plugin-typescript2, but I'm not really sure

There were some bugs related to JSX support, but those might not necessarily have caused issues for all JSX users. Related fixes:

@agilgur5 agilgur5 added kind: bug Something isn't working kind: feature New feature or request labels Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working kind: feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants