Support optional chaining via acorn fork #3582

guybedford · 2020-05-22T05:52:27Z

This PR contains:

Are tests included?

yes (bugfixes and features will not be merged without tests)
no

Breaking Changes?

yes (breaking changes will not be merged unless absolutely necessary)
no

List any relevant issue numbers:

Description

This uses a fork of Acorn I've published with a merged rebase of the optional chaining PR at acornjs/acorn#891.

The fork is available at https://github.com/guybedford/acorn/tree/optional-chaining and I would be glad to keep it maintained there. If you want to move it into the RollupJS org with shared publish permissions we can do that too.

The reason Acorn hasn't landed optional chaining is due to the estree block over how to implement the Node structure. RollupJS doesn't publicly expose the Node interface so I don't believe Rollup as a project should be slowed down by the lack of consensus building work being done here, if it doesn't actually affect Rollup.

There is a huge amount of userland packages and code shipping optional chaining - it's actually incredibly surprising how fast the adoption has been - I believe shipping this will be a huge practical benefit to RollupJS users!

rollup-bot · 2020-05-22T05:57:28Z

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#acorn-optional-chaining

or load it into the REPL:
https://rollupjs.org/repl/?circleci=11455

codecov · 2020-05-22T05:59:02Z

Codecov Report

Merging #3582 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3582   +/-   ##
=======================================
  Coverage   96.35%   96.35%           
=======================================
  Files         180      180           
  Lines        6149     6149           
  Branches     1807     1807           
=======================================
  Hits         5925     5925           
  Misses        111      111           
  Partials      113      113

Impacted Files	Coverage Δ
src/Graph.ts	`100.00% <ø> (ø)`
src/Module.ts	`98.89% <ø> (ø)`
src/ModuleLoader.ts	`100.00% <ø> (ø)`
src/ast/nodes/CallExpression.ts	`95.49% <ø> (ø)`
src/ast/nodes/MemberExpression.ts	`98.27% <ø> (ø)`
src/utils/pureComments.ts	`100.00% <ø> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7285995...6fd914d. Read the comment docs.

lukastaegert · 2020-05-22T07:30:41Z

package.json

@@ -146,5 +145,8 @@
      "default": "./dist/rollup.js"
    },
    "./dist/": "./dist/"
+  },
+  "dependencies": {
+    "fork-acorn-optional-chaining": "^7.2.0"


Please no unnecessary external dependencies. Also why do we need a fork, is it impossible to do this via plugin? I do not feel good about disconnecting from core acorn updates for an unspecified time.

The implementation at https://github.com/acornjs/acorn/pull/891/files is provided as a PR not a plugin. It might be possible to convert the implementation into a plugin, I don't have the resources for that currently though unfortunately.

lukastaegert · 2020-05-22T07:31:26Z

src/Module.ts

@@ -116,7 +116,7 @@ export interface AstContext {
 }

 export const defaultAcornOptions: acorn.Options = {
-	ecmaVersion: 2020,
+	ecmaVersion: 2021 as 2020,


I thought optional chaining was an ES 2020 feature?

It was implemented as 2021 in the PR there, which I'm following - I think you are right though in that it is strictly 2020.

test/form/samples/optional-chaining/_config.js

dilyanpalauzov · 2020-05-25T17:51:46Z

Will this get ”in“, if by 30 June 2020 there is no estree consensus?

lukastaegert · 2020-05-25T18:09:52Z

It will, but there is quite a bit more to be done here. Basically we need to adjust our AST nodes and by the looks of it, create quite a few new ones to make tree-shaking work at all. This is why that is not a straight merge.

kzc · 2020-05-26T04:30:53Z

Do you want to support an acorn fork?

lukastaegert · 2020-05-26T07:30:05Z

Not really, we hope this dilemma will be resolved eventually. But this seems the best way to get it in on the quick.

lukastaegert

It appears my pessimism was not warranted as the merged PR reflects an early stage of the ESTree proposal where it was reusing the existing Nodes. I did not put any work to have improved tree-shaking, but treating those Nodes just as regular MemberExpressions should be a safe starting point.

guybedford · 2020-05-30T23:06:57Z

@lukastaegert shall we move the fork into rollup/acorn here? Let me know and I can start the transfer.

This is not to say I'm not interested in maintaining it - would be glad to help assist with movement here.

lukastaegert · 2020-05-31T03:19:27Z

I fear this might send a wrong message that this fork is maintained in some way. We can wait until there is actually a reason to change something about the fork. I still hope there might be consensus on the ESTree AST and we can get rid of it again.

guybedford · 2020-05-31T04:58:29Z

Good point, we don't want to go the way of Babel's fork :)

tommedema · 2020-08-11T03:01:37Z

I upgraded to rollup 2.23.1 with rollup-plugin-terser 7.0.0 and am getting this error when using optional chaining:

 692 |                 reject(
  693 |                   new Error(
> 694 |                     response?.errorMsg || chrome.runtime.lastError?.message
      |                             ^ Unexpected token: punc (.)
  695 |                   )
  696 |                 );
  697 |               }
[!] (plugin terser) SyntaxError: Unexpected token: punc (.)

lukastaegert · 2020-08-11T03:31:53Z

[!] (plugin terser)

This suggests that it is the terser plugin complaining, not Rollup.

lukastaegert · 2020-08-11T05:07:20Z

I am sorry but I do not understand what you are implying, and what GUI actions have to do with anything. If you are suggesting we fix terser for you, note that this is a completely unrelated project worked on by a completely different set of people. Fixing it sounds rather unsustainable for us because resources are very limited and this is a free open source project maintained by volunteers. As is terser, by the way.

So if you want this fixed, I would suggest you open an issue for that project instead, or even contribute to creating a fix there.

tommedema · 2020-08-11T05:10:11Z

My bad I had posted in the wrong thread, hence the confusion 😉

ohabash · 2022-02-24T15:55:16Z

How can i apply this to fix the issue described here

angular/angular-cli#22743
rollup/plugins#205

numfin · 2022-02-24T17:22:49Z

@ohabash its merged into master. You dont need to checkout fix branch

ohabash · 2022-02-24T18:06:25Z

@numfin Thank you for helping. This must be a terribly obvious answer given all this info but still i must ask... What do I do to fix this?

angular/angular-cli#22743

ohabash · 2022-02-24T19:36:25Z

npm i rollup/rollup results in an error

npm ERR! rollup@2.68.0 postinstall: husky install

is this install what you recommend

numfin · 2022-02-24T21:41:31Z

@numfin Thank you for helping. This must be a terribly obvious answer given all this info but still i must ask... What do I do to fix this?

angular/angular-cli#22743

I dont even understand what are you talking about. What is the connection between angular cli and rollup

ohabash · 2022-02-28T22:21:59Z

@numfin that post describe the issue i have... I prev thought it was a compiler option. But several place online now lead to rollup; specifically this fork. My app wont compile cause of the following error that occurs during an angular build.

✖ Generating FESM2020
Optional chaining cannot appear in left-hand side (Note that you need plugins to import files that are not JavaScript)
[!] Error: unfinished hook action(s) on exit:
(node-resolve) resolveId "./cms.base" "/Users/omar/Sites/met-libs-nx/dist/libs/cms/esm2020/lib/shared/services/flamelink.service.mjs"
(node-resolve) resolveId "../schema-panel/schema-builder.service" "/Users/omar/Sites/met-libs-nx/dist/libs/cms/esm2020/lib/schema-list/schema-list.component.mjs"
...

So im looking for an action item of somekind. Like change a dependancy version or something.

Im sorry but i dont know what next step should be. I think this topic is my best lead though. Thank you for being patient with me..

numfin · 2022-02-28T22:34:32Z

@ohabash dont be sorry, its fine.
Your issue is not rollup related. Its angular related. You did right thing - reporting in angular issues. Or better try to use angular forums and communities. I'm sure you will find someone there who met the same problem. Don't forget to share your configs with them and fully explain the problem step by step: your node version, your angular version, what commands you using and better than everything - repository, where people can reproduce your problem.
Good luck in this problem solving!

ohabash · 2022-02-28T22:54:20Z

I turned on verbose and found a more detailed error... Do you mind taking a look?

This error originates from @rollup@2.68.0

numfin · 2022-02-28T23:10:28Z

@ohabash im not sure this is the best place to talk... :D do you have discord or smth?

ohabash · 2022-02-28T23:28:21Z

https://discord.gg/AQzAfBcS @numfin

ohabash · 2022-03-02T18:03:32Z

This was my solution ng-packagr/ng-packagr#2259 (comment)

lukastaegert requested changes May 22, 2020

View reviewed changes

guybedford force-pushed the acorn-optional-chaining branch from fd6ad48 to f6d4333 Compare May 23, 2020 07:44

lukastaegert approved these changes May 27, 2020

View reviewed changes

lukastaegert force-pushed the acorn-optional-chaining branch from 192f65a to 62224b1 Compare May 27, 2020 18:41

guybedford and others added 5 commits May 27, 2020 20:51

support optional chaining via acorn fork

3c560de

fixup test

6627dd3

fixup dep

62aca02

Make acorn a devDependency again

f215ab3

Modify test to see at least basic tree-shaking works.

d479801

lukastaegert force-pushed the acorn-optional-chaining branch from 62224b1 to d479801 Compare May 27, 2020 18:52

Update dependencies and do not bundle acorn twice

6fd914d

lukastaegert merged commit ad6c24a into master May 27, 2020

lukastaegert deleted the acorn-optional-chaining branch May 27, 2020 19:31

lukastaegert mentioned this pull request May 27, 2020

Optional chaining et al #3469

Closed

jespertheend mentioned this pull request Jun 4, 2020

Optional Chaining Causes Build to Fail rollup/plugins#205

Closed

evanw mentioned this pull request Jul 5, 2020

import.meta doesn't work with esbuild since 0.5.17 vitejs/vite#503

Closed

underfin mentioned this pull request Jul 17, 2020

Error [SyntaxError]: Unexpected token: punc (.), During Build, when using optional chaining in the code vitejs/vite#565

Closed

abdonrd mentioned this pull request Mar 12, 2021

chore(deps): remove unneeded babel plugins open-wc/open-wc#2111

Merged

ohabash mentioned this pull request Feb 28, 2022

"Optional chaining cannot appear in left-hand side" Suddenly appeared. angular/angular-cli#22743

Closed

ohabash mentioned this pull request Mar 1, 2022

cant build due to "Optional chaining cannot appear in left-hand side" ng-packagr/ng-packagr#2259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support optional chaining via acorn fork #3582

Support optional chaining via acorn fork #3582

guybedford commented May 22, 2020

rollup-bot commented May 22, 2020 •

edited

codecov bot commented May 22, 2020 •

edited

lukastaegert May 22, 2020

guybedford May 22, 2020

lukastaegert May 22, 2020

guybedford May 22, 2020

dilyanpalauzov commented May 25, 2020

lukastaegert commented May 25, 2020

kzc commented May 26, 2020

lukastaegert commented May 26, 2020

lukastaegert left a comment

guybedford commented May 30, 2020

lukastaegert commented May 31, 2020

guybedford commented May 31, 2020

tommedema commented Aug 11, 2020 •

edited

lukastaegert commented Aug 11, 2020

lukastaegert commented Aug 11, 2020

tommedema commented Aug 11, 2020

ohabash commented Feb 24, 2022

numfin commented Feb 24, 2022

ohabash commented Feb 24, 2022

ohabash commented Feb 24, 2022

numfin commented Feb 24, 2022

ohabash commented Feb 28, 2022 •

edited

numfin commented Feb 28, 2022

ohabash commented Feb 28, 2022 •

edited

numfin commented Feb 28, 2022

ohabash commented Feb 28, 2022 •

edited

ohabash commented Mar 2, 2022

Support optional chaining via acorn fork #3582

Support optional chaining via acorn fork #3582

Conversation

guybedford commented May 22, 2020

Description

rollup-bot commented May 22, 2020 • edited

Thank you for your contribution! ❤️

codecov bot commented May 22, 2020 • edited

Codecov Report

lukastaegert May 22, 2020

Choose a reason for hiding this comment

guybedford May 22, 2020

Choose a reason for hiding this comment

lukastaegert May 22, 2020

Choose a reason for hiding this comment

guybedford May 22, 2020

Choose a reason for hiding this comment

dilyanpalauzov commented May 25, 2020

lukastaegert commented May 25, 2020

kzc commented May 26, 2020

lukastaegert commented May 26, 2020

lukastaegert left a comment

Choose a reason for hiding this comment

guybedford commented May 30, 2020

lukastaegert commented May 31, 2020

guybedford commented May 31, 2020

tommedema commented Aug 11, 2020 • edited

lukastaegert commented Aug 11, 2020

lukastaegert commented Aug 11, 2020

tommedema commented Aug 11, 2020

ohabash commented Feb 24, 2022

numfin commented Feb 24, 2022

ohabash commented Feb 24, 2022

ohabash commented Feb 24, 2022

numfin commented Feb 24, 2022

ohabash commented Feb 28, 2022 • edited

numfin commented Feb 28, 2022

ohabash commented Feb 28, 2022 • edited

numfin commented Feb 28, 2022

ohabash commented Feb 28, 2022 • edited

ohabash commented Mar 2, 2022

rollup-bot commented May 22, 2020 •

edited

codecov bot commented May 22, 2020 •

edited

tommedema commented Aug 11, 2020 •

edited

ohabash commented Feb 28, 2022 •

edited

ohabash commented Feb 28, 2022 •

edited

ohabash commented Feb 28, 2022 •

edited