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

Ambient const enums are not allowed when the '--isolatedModules' flag is provided #7110

Closed
gsbelarus opened this issue Nov 15, 2018 · 24 comments · Fixed by #8005
Closed

Ambient const enums are not allowed when the '--isolatedModules' flag is provided #7110

gsbelarus opened this issue Nov 15, 2018 · 24 comments · Fixed by #8005
Assignees

Comments

@gsbelarus
Copy link

Project created with create-react-app 2.1 with TS 3.1.6 is not compiling and falling with error:

yarn build

yarn run v1.12.3
$ react-scripts build
Creating an optimized production build...
Failed to compile.

.../node_modules/@uifabric/utilities/lib/KeyCodes.d.ts
Type error: Ambient const enums are not allowed when the '--isolatedModules' flag is provided. TS1209

4 |  * @public
5 |  */

6 | export declare const enum KeyCodes {
| ^
7 | backspace = 8,
8 | tab = 9,
9 | enter = 13,

Project dependencies:

  "dependencies": {
    "@types/dagre": "^0.7.40",
    "@types/jest": "^23.3.9",
    "@types/node": "^10.12.5",
    "@types/react": "^16.7.3",
    "@types/react-dom": "^16.0.9",
    "dagre": "^0.8.2",
    "office-ui-fabric-react": "^6.102.0",
    "react": "^16.6.1",
    "react-dom": "^16.6.1",
    "react-redux": "^5.1.1",
    "react-router-dom": "^4.3.1",
    "react-scripts": "2.1.1",
    "redux": "^4.0.1",
    "redux-devtools-extension": "^2.13.5",
    "redux-thunk": "^2.3.0",
    "typesafe-actions": "^2.0.4",
    "typescript": "^3.1.6"
  },
@kenotron
Copy link
Member

We're actually in the middle of addressing this issue. Half of it landed last night: a995de2

I'm working on the icon names and const enums across more packages now.

@kenotron
Copy link
Member

If you feel that this is resolved, please feel free to close this. We're taking an approach of carefully moving over to something other than const enum trying to figure out the implications of size differences in the package before and after. It'll likely take a few more days to completely land.

@cschleiden
Copy link
Contributor

I'm hitting this right now, should this be re-opened?

@gsbelarus
Copy link
Author

If you are on latest version and there are still global exported enums, than definitely should be.

PS: until they are cooperating with TS guys to address this problem on TS behalf.

@cschleiden cschleiden reopened this Nov 19, 2018
@cschleiden
Copy link
Contributor

@kenotron I'm hitting this with the latest packages right now because of the icon names. You mentioned that you'd fixed part 1 of 2 so if there is another issue tracking part 2 please feel free to resolve as dupe.

@kenotron
Copy link
Member

Hey @cschleiden - yeah we haven't finished the work yet, though you can track it here: #7119

@dushes-ag
Copy link

Hi is there any progress on the matter? still same behavior. Seems to be critical as it totally blocks using the fabricui in react+typescript app

@Jahnp
Copy link
Member

Jahnp commented Dec 12, 2018

@dushes-ag : @kenotron and I will be tackling the matter of IconNames as that should be the last outstanding const enum in the library, but it's a little trickier to handle because of the devexp of using IconNames right now.

That said, in the meantime, const enum should not block use of Fabric in React + TypeScript apps generally. It is because Create React App's baked-in TypeScript support uses Babel, which doesn't support const enum. So if you use another method of creating a React app you shouldn't be blocked.

One way around this that lets you still use CRA is to use react-scripts-ts (we use this on the Getting Started page):

npx create-react-app my-app --scripts-version=react-scripts-ts

@drewloomer
Copy link

@Jahnp is there any other known work around since react-scripts-ts is deprecated?

@Jahnp
Copy link
Member

Jahnp commented Dec 13, 2018

@drewloomer oh shoot, I didn't realize that was the case. I'm not aware of another work around right now. We'll have to get back to you on that.

There's a decent chance we'll be removing the const enum from IconNames over the next few weeks altogether, so the matter should sort itself out soon.

@BTC-Bradley
Copy link
Contributor

@drewloomer What I'm doing as a work around is going to the IconNames.d.ts file within node_modules and I'm simply removing the const from the enum. This should suffice for development purposes until the team fully deprecates this last const enum.

@chriskuech
Copy link

I was able to suppress this without making code changes to prepackaged code by setting "skipLibCheck": true" in my tsconfig.json

@micahgodbolt
Copy link
Member

@Jahnp It looks like the only const enum issue left is the icons, correct? Can we open up an issue specifically for icons and close this one up?

@SudharsanSukumar
Copy link

Any updates on this issue?

@kenotron
Copy link
Member

kenotron commented Feb 14, 2019

@BTC-Bradley - after a lot of thinking on this issue, we're gonna deprecate the IconNames module. Most of consumers are using <Icon> with a plain string name that is taken from the tool:

https://uifabricicons.azurewebsites.net/?help

It's not the resolution you probably want, but in a rare occasion I'm actually recommending against having a real type here so far. We can probably do better in a later major rev of Fabric where we could export little constants (e.g. export const FooIconName='FooIcon'). The #8003 issue should be addressed by adding a @deprecated flag in IconNames.

@kenotron
Copy link
Member

Ooops, just saw @JasonGore marking the newer bug resolved. I'm gonna keep this one open for the @deprecated work

@msft-github-bot
Copy link
Contributor

🎉This issue was addressed in #8005, which has now been successfully released as @uifabric/[email protected].:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉This issue was addressed in #8005, which has now been successfully released as [email protected].:tada:

Handy links:

@BTC-Bradley
Copy link
Contributor

Thanks @kenotron for the quick response!

@BTC-Bradley
Copy link
Contributor

@kenotron, would it also be possible to remove the const from the enum since it's deprecated? This would resolve the error for this issue until the enum is completely removed.

enum

@rhalaly
Copy link

rhalaly commented Apr 9, 2019

@fabricteam can you please assist with BTC-Bradley issue. I'm also facing it...

@hkhamm
Copy link

hkhamm commented Apr 10, 2019

I still have the IconNames issue using "office-ui-fabric-react": "^6.164.8". The only fix that works is to remove the const.

@BTC-Bradley
Copy link
Contributor

@rhalaly @hkhamm -

The @fabricteam has marked the enum as deprecated and I've been told it is set to be removed in the next major release 7.0.

Until then, I have been using a simple (kinda hacky) script in my React app to automatically remove the const before building/starting. Sharing here in case anyone finds it useful:

Add replace-in-file to dev dependencies:

yarn add --dev replace-in-file

Paste this code to scripts/prebuild.js file:

const replace = require('replace-in-file');
const fabricOptions = {
  files: './node_modules/@uifabric/icons/lib/IconNames.d.ts',
  from: /const\s/g,
  to: '',
};

try {
  const changes = replace.sync(fabricOptions);
  if (changes.length > 0) {
    console.log('Modified files: ', changes.join(', '));
    console.log('const Removed! :)');
  } else {
    console.log('const was already removed! :)');
  }
}
catch (err) {
  console.error('Error occurred while modifying IconNames, remove manually');
  return console.error(err);
}

Modify package.json:

"start": "yarn pre && react-scripts start",
"build": "yarn pre && react-scripts build",
"pre": "node scripts/prebuild.js"

@jeremy-coleman
Copy link

just change the = to : and export as a type
export type IconNames = {
GlobalNavButton: 'GlobalNavButton',

@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.