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

Optional Chaining Causes Build to Fail #205

Closed
bennypowers opened this issue Feb 9, 2020 · 16 comments
Closed

Optional Chaining Causes Build to Fail #205

bennypowers opened this issue Feb 9, 2020 · 16 comments

Comments

@bennypowers
Copy link
Contributor

bennypowers commented Feb 9, 2020

  • Rollup Plugin Name: @rollup/plugin-typescript
  • Rollup Plugin Version: ^3.0.0
  • Rollup Version: ^1.31.0
  • Operating System (or Browser): Mac OS
  • Node Version: v12.3.1

How Do We Reproduce?

git clone git@github.com:bennypowers/rollup-plugin-typescript-optional-chaining-repro.git
cd rollup-plugin-typescript-optional-chaining-repro
npm i
npx rollup -c

Expected Behavior

Compiles

Actual Behavior

Fails with

index.ts → stdout...
[!] Error: Unexpected token (Note that you need plugins to import files that are not JavaScript)
index.ts (2:3)
1: const a = { b: { c: undefined } };
2: a?.b?.c;
      ^
Error: Unexpected token (Note that you need plugins to import files that are not JavaScript)
    at error (/Users/bennyp/Documents/rt-repro/node_modules/rollup/dist/shared/node-entry.js:5400:30)
    at Module.error (/Users/bennyp/Documents/rt-repro/node_modules/rollup/dist/shared/node-entry.js:9820:16)
    at tryParse (/Users/bennyp/Documents/rt-repro/node_modules/rollup/dist/shared/node-entry.js:9713:23)
    at Module.setSource (/Users/bennyp/Documents/rt-repro/node_modules/rollup/dist/shared/node-entry.js:10076:33)
    at /Users/bennyp/Documents/rt-repro/node_modules/rollup/dist/shared/node-entry.js:12362:20
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at async Promise.all (index 0)

I now present the relevant contents of said repro repo

package.json

{
  "devDependencies": {
    "@rollup/plugin-typescript": "^3.0.0",
    "rollup": "^1.31.0",
    "typescript": "^3.7.5"
  }
}

rollup.config.js

import typescript from '@rollup/plugin-typescript';

export default {
  input: 'index.ts',
  output: {
    format: 'es',
    outDir: '.'
  },
  plugins: [typescript({ typescript: require('typescript') })]
}

index.ts

const a = { b: { c: undefined } };
a?.b?.c;

tsconfig.json

{
  "include": ["index.ts"],
  "compilerOptions": {
    "experimentalDecorators": true,
    "target": "esnext",
    "module": "esnext"
  }
}
@bennypowers
Copy link
Contributor Author

I was able to reproduce with this test:

test('supports optional chaining', async (t) => {
  const bundle = await rollup({
    input: 'fixtures/optional-chaining/main.ts',
    plugins: [typescript({ module: 'esnext', target: 'esnext' })],
    onwarn
  });
  const output = await evaluateBundle(bundle);
  t.is(output, 'NOT FOUND');
});

fixtures/optional-chaining/main.ts:

interface OC {
  a: number;
  b?: {
    c?: number;
  };
}
const o = { a: 1 } as OC;
export default o.b?.c ?? 'NOT FOUND';
  1 test failed

  supports optional chaining

  /Users/bennyp/Documents/plugins/node_modules/.pnpm/registry.npmjs.org/rollup/1.29.0/node_modules/rollup/dist/rollup.js:5340

  Rejected promise returned by test. Reason:

  Error {
    code: 'PARSE_ERROR',
    frame: `6: }␊
    7: const o = { a: 1 } as OC;␊
    8: export default o.b?.c ?? 'NOT FOUND';␊
                           ^`,
    loc: {
      column: 20,
      file: '/Users/bennyp/Documents/plugins/packages/typescript/test/fixtures/optional-chaining/main.ts',
      line: 8,
    },
    parserError: SyntaxError {
      loc: Position {
        column: 19,
        line: 2,
      },
      pos: 39,
      raisedAt: 40,
      message: 'Unexpected token (2:19)',
    },
    pos: 39,
    watchFiles: [
      '/Users/bennyp/Documents/plugins/packages/typescript/test/fixtures/optional-chaining/main.ts',
    ],
    message: 'Unexpected token (Note that you need plugins to import files that are not JavaScript)',
  }

  error (/Users/bennyp/Documents/plugins/node_modules/.pnpm/registry.npmjs.org/rollup/1.29.0/node_modules/rollup/dist/rollup.js:5340:30)
  Module.error (/Users/bennyp/Documents/plugins/node_modules/.pnpm/registry.npmjs.org/rollup/1.29.0/node_modules/rollup/dist/rollup.js:9730:9)
  tryParse (/Users/bennyp/Documents/plugins/node_modules/.pnpm/registry.npmjs.org/rollup/1.29.0/node_modules/rollup/dist/rollup.js:9636:16)
  Module.setSource (/Users/bennyp/Documents/plugins/node_modules/.pnpm/registry.npmjs.org/rollup/1.29.0/node_modules/rollup/dist/rollup.js:9986:33)
  /Users/bennyp/Documents/plugins/node_modules/.pnpm/registry.npmjs.org/rollup/1.29.0/node_modules/rollup/dist/rollup.js:12265:20

/Users/bennyp/Documents/plugins/packages/typescript:
 ERROR  @rollup/plugin-typescript@3.0.0 test: `ava`
Exit status 1

@luwes
Copy link

luwes commented Feb 10, 2020

related to this @rollup/plugin-commonjs also breaks on this syntax.

should work I think, use case on our end is that we have a --faster build which excludes Babel and outputs a modern ES bundle which works on the latest Chrome.

@shellscape
Copy link
Collaborator

/cc @NotWoods

@NotWoods NotWoods self-assigned this Feb 10, 2020
@bennypowers
Copy link
Contributor Author

I might not be able to contribute the fix, but if it's alright with you, may I open a PR with the tests (for maintainers to finish up?)

@NotWoods
Copy link
Member

Yes, that would be great!

@NotWoods
Copy link
Member

I've done some investigation and I think that Rollup/acorn is having trouble parsing the new optional chaining syntax. If you set target: 'es2020' (so that Typescript compiles the syntax with a helper) the issue goes away.

@bennypowers
Copy link
Contributor Author

If you set target: 'es2020' (so that Typescript compiles the syntax with a helper) the issue goes away.

I was unable to reproduce this. I tried setting target both in tsconfig and in rollup config

    typescript({ target: 'ES2020' }),

@NotWoods
Copy link
Member

In that case it should start working in the next major version. I've been working on another rewrite to support .d.ts and I have optional chaining working there.

@NotWoods
Copy link
Member

NotWoods commented Mar 6, 2020

Fixed in version 4.0.0 of the typescript plugin

@NotWoods NotWoods closed this as completed Mar 6, 2020
@etroynov
Copy link

I use v4.0.0 and problem still exist :(

@etroynov
Copy link

i found the solution, in tsconfig i change:

target esnext -> es2019

For some reason optional chaining does not work with 2020, esnext targets.

@numfin
Copy link

numfin commented Mar 14, 2020

@etroynov it is a problem in rollup, not in typescript
@NotWoods it should be possible to use optional chaining without other compilers

@shellscape
Copy link
Collaborator

@numfin please combine multiple successive replies so we keep the notifications down.

yyx990803 pushed a commit to vitejs/vite that referenced this issue May 17, 2020
Rollup does not appear to support optional chaining (`foo?.bar`). Work
around this by lowering the esbuild target to ES2019

Related: rollup/plugins#205
@jespertheend
Copy link

@numfin this seems to have been fixed in rollup/rollup#3582

@Robin-Hoodie
Copy link

For some reason optional chaining does not work with 2020, esnext targets.

Isn't this expected behavior? The optional chaining operator is part of ES 2020, hence it doesn't need to be downcompiled.

@ohabash

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants