storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.02k stars 9.23k forks source link

TS1086: An accessor cannot be declared in an ambient context #9463

Closed cloakedninjas closed 4 years ago

cloakedninjas commented 4 years ago

Describe the bug Upgraded to 5.3.3 just now and our Angular build is failing with the following:

[ng]
[ng] ERROR in ../../node_modules/@storybook/channels/dist/index.d.ts:25:9 - error TS1086: An accessor cannot be declared in an ambient context.
[ng]
[ng] 25     get hasTransport(): boolean;
[ng]            ~~~~~~~~~~~~

From a quick google, it appears other repos are facing the same issue due to the use of TypeScript 3.7.0 - but Angular is restricted to <3.5.0

Similar errors https://github.com/nestjs/nest/issues/3513 https://github.com/googleapis/node-gtoken/issues/244

I believe the breaking change come from TypeScript directly https://github.com/microsoft/TypeScript/issues/33939 - but until a fix is released, the index.d.ts file will need regenerating with npm i typescript@~3.6.0 (swap ^ for ~)

System:

    OS: macOS Mojave 10.14.6
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 12.14.1 - /usr/local/bin/node
    npm: 6.13.4 - /usr/local/bin/npm
  Browsers:
    Chrome: 79.0.3945.117
    Safari: 13.0.4
  npmPackages:
    @storybook/addon-actions: ^5.3.3 => 5.3.3
    @storybook/addon-knobs: ^5.3.3 => 5.3.3
    @storybook/addon-links: ^5.3.3 => 5.3.3
    @storybook/addon-notes: ^5.3.3 => 5.3.3
    @storybook/addon-storyshots: ^5.3.3 => 5.3.3
    @storybook/addons: ^5.3.3 => 5.3.3
    @storybook/angular: ^5.3.3 => 5.3.3
    @storybook/cli: ^5.3.3 => 5.3.3
cloakedninjas commented 4 years ago

I had a stab at doing the upgrade but got lost in the package structure πŸ˜… - The end result (after dist files are generated) should make @storybook/channels/dist/index.d.ts:25 go

From

get hasTransport(): boolean;

To

readonly hasTransport: boolean;
siropo commented 4 years ago

I had the same issue. I resolve it by update the Typescript package to the latest version

IsharaMadawa commented 4 years ago

I had the same issue. I resolve it by update the Typescript package to the latest version

@siropo What version? i'm using angular 8.3.20 and typescript 3.5.3. And i got same error when try to build the project.

siropo commented 4 years ago

@IsharaMadawa "typescript": "3.7.4"

IsharaMadawa commented 4 years ago

@IsharaMadawa "typescript": "3.7.4"

@siropo Can you drop the package.json here?

siropo commented 4 years ago

@IsharaMadawa why? Just try to update your typescript version and check if it is working

cloakedninjas commented 4 years ago

If you're running older version of Angular, e.g. 8.1.0 you'll see the following error

The Angular Compiler requires TypeScript >=3.4.0 and <3.5.0 but 3.x.y was found instead.

IsharaMadawa commented 4 years ago

If you're running older version of Angular, e.g. 8.1.0 you'll see the following error

The Angular Compiler requires TypeScript >=3.4.0 and <3.5.0 but 3.x.y was found instead.

yes. i got this error and had to update angular also

yarsunny commented 4 years ago

@IsharaMadawa Did you upgrade to angular 9 rc version ? I see the below error The Angular Compiler requires TypeScript >=3.4.0 and <3.5.0 but 3.x.y was found instead. in angular 8.2.3 as well

konstantintieber commented 4 years ago

This also breaks for us using the latest version of Angular 8.2.x :/

shivarajnaidu commented 4 years ago

image

flvanroekel commented 4 years ago

I have the 3.7.4 version of Typescript and the 8.3.21 version of Angular and I am getting the same message! Please help! :-)

alex-vas commented 4 years ago

Updating with ng update --next @angular/cli --force to 9.0.0-rc.12 which uses typescript @ "3.7.5", has helped the issue indeed.

mshibl commented 4 years ago

Getting this same issue with typescript 3.7.5 and storybook/react 5.3.9

Snargol commented 4 years ago

ng update --next @angular/cli --force npm install typescript@latest

;)

Janaki96 commented 4 years ago

ng update --next @angular/cli --force npm install typescript@latest

;) It worked,Thank you Snargol.

cloakedninjas commented 4 years ago

Confirmed, upgrading to Angular 9 (and by extension Typescript 3.7.5), this issue is resolved

lychyi commented 4 years ago

Upgrading to TypeScript 3.7.x isn't a great option for library maintainers. This means that the change from 3.6 to 3.7 (technically a breaking change) gets passed down to our consumers and we can't assume they can readily upgrade TypeScript either.

@ndelangen I think this can be rectified by changing https://github.com/storybookjs/storybook/blob/next/package.json#L218 to typescript@~3.6.0, since typescript@^3.4.0 resolves to 3.7.5 right now. Then we can save the 3.7 upgrade until until Storybook 6.0 since it's actually a breaking change.

What do you think?

ndelangen commented 4 years ago

Sounds like a plan @lychyi

Want to open a PR against the next branch?

We'll cherry-pick it into master and release as a patch @shilman

lychyi commented 4 years ago

You got it!

lychyi commented 4 years ago

@ndelangen @shilman

Should Storybook 6 utilize TypeScript 3.7 since it's going to be offered as a breaking change anyway? If so, I think this should actually get changed in master and not next. The current PR #9826 is against next but I can rehash that to point to master instead, let me know.

Consequently, if TS 3.7+ is not really important, it could be upgraded as part of Storybook 7 or beyond as well?

shilman commented 4 years ago

works for me @lychyi. thanks so much for taking care of this & thinking it through!!! πŸ’―

re: 3.7 in storybook 6.0. i'm not sure about whether 3.7 is necessary, will let one of the typescript guys speak to that @ndelangen @kroeder @gaetanmaisse

lychyi commented 4 years ago

FWIW, next already utilizes 3.7, which to its merit, has really neat features like optional chaining (foo?.bar) and the nullish coalescing operator (foo ?? bar). Changing this only in master for Storybook 5.3.x would be the least disruptive to the current state of things.

Also, I just learned that the getter/setter emit is only a breaking change for "typescript": "<=3.5". TS 3.6 future-proofed this feature so another option is to have people upgrade to TS3.6 if they can't upgrade to TS3.7 for whatever reason. Breaking changes in TS3.6 is not nearly as bad.

However, using TS3.7 still hurts Angular folks not able to upgrade to 9.x yet. Angular 8.x is locked at ~3.5.3.

So the main tradeoff I suggested weighing in on is: "Would we be comfortable saying that Storybook 6 is not compatible with Angular 8 so that we could leverage the latest and greatest that TypeScript 3.7 has to offer?"

shilman commented 4 years ago

@lychyi Great summary. Uninformed gut feel says definitely 3.7 for 6.0. Seize the future!! 😈

gaetanmaisse commented 4 years ago

@lychyi Thanks for the great work! πŸ‘ πŸ‘

Would we be comfortable saying that Storybook 6 is not compatible with Angular 8 so that we could leverage the latest and greatest that TypeScript 3.7 has to offer?

For me, it's a bit harsh to make SB 6 not compatible with Angular < 9 because Angular 9 is out for only a few days. Many packages aren't ready for this new version, which for Angular newbie is a real major version that introduces a new compilation and rendering pipeline, and so migrating to Angular 9 cannot be done for some big monorepo project (like the one I working with at work).

As enhancements bring by TS 3.7 are no currently widely used in SB monorepo we can maybe keep TS 3.6 (so merge the PR in next + cherry pick on master) until we find a way to work around this issue. Maybe a project to take a look at: https://github.com/sandersn/downlevel-dts it allows generating old definition files based on new ones πŸ€·β€β™‚

shilman commented 4 years ago

@gaetanmaisse You're much more in touch with the TS/Angular world, so ultimately I'll go with your judgement on this. However, I want to point out a few things: 1) Current estimate on SB6 is end of April and by that time NG9 will be a few months old 2) If this is indeed a breaking change and we don't change it now, the next time we'll be able to change it is 7.0, which will likely be a year off.

Of course, if we can get it fixed with 3.6 then that's even better! Inside Storybook it'd be great to start using optional chaining etc., tho I suppose that's relevant to the dev dependency and not the dependency.

gaetanmaisse commented 4 years ago

@shilman I totally agree with you πŸ’― and I'm quite disappointed to have to keep 3.6 until SB7.

As SB6 is still in alpha for weeks I think we should say that our goal is to have TS 3.7+ in SB6 and find a way to still be compatible with Angular <= 8. And if it's not possible, see at the end of March or the beginning of Apris how NG9 has been adopted and discuss about dropping support of Angular <= 8 support.

lychyi commented 4 years ago

@gaetanmaisse I'm playing around with a branch to utilize downlevel-dts. At first I dismissed it because it's not a maintained part of TypeScript but after thinking through it a bit, it could be a viable option to let us use TS3.7+ and still be compatible with <=TS3.5 users. Give me a bit to verify the changes and then we can see if it's worth the maintenance. The build scripts are pretty tidy so this shouldn't be too difficult.

lychyi commented 4 years ago

@gaetanmaisse PR is up, let me know if you have any questions.

Thanks for the opportunity to contribute!

mihaisitaru commented 4 years ago

Updating with ng update --next @angular/cli --force to 9.0.0-rc.12 which uses typescript @ "3.7.5", has helped the issue indeed.

This solved my issue too! Thanks a lot!

ColCh commented 4 years ago

Ok but how to resolve this issue without typescript update?

gaetanmaisse commented 4 years ago

@ColCh we are working on it https://github.com/storybookjs/storybook/pull/9847 it should be available soon! πŸ‘ πŸ‘ @lychyi

ColCh commented 4 years ago

Thank you! That PR seem to fix the issue.

Ok, patiently waiting for it :) no hurry

ColCh commented 4 years ago

Wow that was fast!

But, @gaetanmaisse , may we expect this commit to be backported to v5 version? That would be nice

gaetanmaisse commented 4 years ago

@ColCh It will! We are testing that everything is OK on the latest 6.0.0-alpha and then we will release it in a 5.3.x version πŸ˜‰

ColCh commented 4 years ago

Oh, okay. Thank you πŸ™‚

cnestor1 commented 4 years ago

I have the following configuration and I am still getting the error:

`"

 @angular/cli": "7.3.8",
"@angular/compiler-cli": "7.2.8",
"@angular/language-service": "7.2.3",
"@babel/core": "^7.7.7",
"@babel/preset-env": "^7.8.4",
"@babel/preset-typescript": "^7.8.3",
"@ngx-translate/core": "11.0.0",
"@storybook/addon-actions": "5.3.13",
"@storybook/addon-links": "5.3.13",
"@storybook/addon-notes": "5.3.13",
"@storybook/addon-storyshots": "^5.3.13",
"@storybook/addon-viewport": "^5.3.13",
"@storybook/addons": "^5.3.13",
"@storybook/angular": "5.3.13",
 "typescript": "^3.2.4"`

ng build --prod ERROR in node_modules/@storybook/channels/dist/index.d.ts(25,9): error TS1086: An accessor cannot be declared in an ambient context.

gaetanmaisse commented 4 years ago

@cnestor1 the bugfix has not been released in 5.3.x yet, it should be in the next one πŸ˜‰

cnestor1 commented 4 years ago

Thx, I misunderstood and thought it had been. What's your ETA for the next release? Also is there a way to use an older version of Storybook to circumvent the issue until it's fixed? Angular 9 hasn't been vetted in my company, so I have to wait a bit before I can upgrade.

shilman commented 4 years ago

@cnestor1 ETA in the next 5 days

ghost commented 4 years ago

I resolved this error temporarily by visiting the nodemodules file (.d.ts file)mentioned in the error and then saving it by ctrl+s this will resolve error as your code will be compiled but error will still be there

shilman commented 4 years ago

Son of a gun!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.14 containing PR #9847 that references this issue. Upgrade today to try it out!

Closing this issue. Please re-open if you think there's still more to do.

rohittagadiya commented 4 years ago

Getting same issue :)

shilman commented 4 years ago

@rohittagadiya did you update to latest?

pankajdaffodil commented 4 years ago

Try "skipLibCheck": true, inside tsconfig.json eg.

"compilerOptions": {
    "baseUrl": "src",
    "outDir": "./dist/out-tsc",
    "sourceMap": true,
    "declaration": false,
    "downlevelIteration": true,
    "experimentalDecorators": true,
    "module": "esnext",
    "moduleResolution": "node",
    "importHelpers": true,
    "target": "es2015",
    **"skipLibCheck": true,**
    "typeRoots": [
      "node_modules/@types"
    ],
    "lib": [
      "es2018",
      "dom"
    ]
  },

Hope it will help you !!

P-dro commented 4 years ago

Tente " skipLibCheck": true , dentro do tsconfig.json, por exemplo.

"compilerOptions": {
    "baseUrl": "src",
    "outDir": "./dist/out-tsc",
    "sourceMap": true,
    "declaration": false,
    "downlevelIteration": true,
    "experimentalDecorators": true,
    "module": "esnext",
    "moduleResolution": "node",
    "importHelpers": true,
    "target": "es2015",
    **"skipLibCheck": true,**
    "typeRoots": [
      "node_modules/@types"
    ],
    "lib": [
      "es2018",
      "dom"
    ]
  },

Espero que ajude vocΓͺ !!

Works!!!! Thanks

LuisHCK commented 4 years ago

Try "skipLibCheck": true, inside tsconfig.json eg.

"compilerOptions": {
    "baseUrl": "src",
    "outDir": "./dist/out-tsc",
    "sourceMap": true,
    "declaration": false,
    "downlevelIteration": true,
    "experimentalDecorators": true,
    "module": "esnext",
    "moduleResolution": "node",
    "importHelpers": true,
    "target": "es2015",
    **"skipLibCheck": true,**
    "typeRoots": [
      "node_modules/@types"
    ],
    "lib": [
      "es2018",
      "dom"
    ]
  },

Hope it will help you !!

Works pretty well, Thanks <3

deborahjames commented 4 years ago

Adding "skipLibCheck": true, in the compilerOptions inside file tsconfig.json worked for me. Thank you so much .

switch120 commented 4 years ago

Chiming in - skipLibCheck: true works a charm. This is great for those who can't risk an ng update mid-QA cycle. Thanks!

shafeek2112 commented 4 years ago

Try "skipLibCheck": true, inside tsconfig.json eg.

"compilerOptions": {
    "baseUrl": "src",
    "outDir": "./dist/out-tsc",
    "sourceMap": true,
    "declaration": false,
    "downlevelIteration": true,
    "experimentalDecorators": true,
    "module": "esnext",
    "moduleResolution": "node",
    "importHelpers": true,
    "target": "es2015",
    **"skipLibCheck": true,**
    "typeRoots": [
      "node_modules/@types"
    ],
    "lib": [
      "es2018",
      "dom"
    ]
  },

Hope it will help you !!

Thank you. Working well.