Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

const enums are not inlined #400

Open
axelo opened this issue Sep 15, 2018 · 6 comments
Open

const enums are not inlined #400

axelo opened this issue Sep 15, 2018 · 6 comments

Comments

@axelo
Copy link

axelo commented Sep 15, 2018

Is this a bug report?

Yes.

const enums are not inlined when building (npm build / yarn build).

The reason can be understood here microsoft/TypeScript#11202

And the problem/feature is this line https://github.com/wmonk/create-react-app-typescript/blob/master/config/webpack.config.prod.js#L185

Our use case:
A large const enum that is generated and only a few values are actually used resulting in our bundle size to be a lot bigger than expected (+ 40kb). Plus we would like to think the values used to be just as strings while running and not accessed through an object.

Can you also reproduce the problem with npm 4.x?

Didn't seem relevant.

Which terms did you search for in User Guide?

const, enum, transpile

Environment

  1. npm ls react-scripts-ts (if you haven’t ejected):
    react-scripts-ts@2.17.0

  2. node -v:
    v8.11.1

  3. npm -v:
    5.6.0

  4. yarn --version (if you use Yarn):
    1.9.4

Then, specify:

  1. Operating system:
    macOS High Sierra

Steps to Reproduce

  1. yarn create react-app my-app --scripts-version=react-scripts-ts
  2. create an enum in the source directory
export const enum MyConstEnum {
  One = 'One',
  Two = 'Two'
}

Import the enum in app and only use 'One'.

import * as React from 'react';
import { MyConstEnum } from './MyConstEnum';

class App extends React.Component {
  public render() {
    return (
      MyConstEnum.One
    );
  }
}

export default App;
  1. yarn build
  2. Inspect build/static/js/main*.js seach for "One" and we find
    function(e){e.One="One",e.Two="Two"}(r||(r={}))}
    And
    t.prototype.render=function(){return i.a.One}
    instead of the inlined version

Expected Behavior

If we manually changes the transpile flag to false and run yarn build again and inspects main.js

t.prototype.render=function(){return"One"}

And we got the inlined version :)

Actual Behavior

We uses the non inlined version (as described in step 5)

Reproducible Demo

Steps to Reproduce should be enough, otherwise I can push a demo project.

@DorianGrey
Copy link
Collaborator

An interesting aspect, at least. Using transpileOnly allows to perform type-checking in parallel to the compilation (here: using fork-ts-checker-webpack-plugin), so it is faster than doing both at once.

However, most of this benefit goes to development mode. Production builds are faster as well, yet it might be acceptable to slow these down a bit in favor of correctness. Thus, it might be worth to disable that flag and the plugin for build mode. Just thinking about what to do with the linter execution in this case, which is also performed by the plugin (the linter execution is optional, but the type-checker cannot be disabled without disabling both).

@axelo
Copy link
Author

axelo commented Sep 15, 2018

We also uses const enums for redux-actions so some bytes could be saved for other projects (no data on this though). But maybe this isn't that big of a problem for most projects, and maybe some information about the trade off, and if possible, an optional flag, with the effect of slowing down the build a bit.

@evaera
Copy link

evaera commented Sep 27, 2018

Note that const enums may appear in declaration files. Some modules may accept either literal strings or (even worse) special numbers with significant meaning. When writing type declarations, creating const enums for these situations is ideal because you get proper tooling while still being able to be compatible with the source library.

Because of that, if these enums are not inlined properly (even in development mode!) the transpilation step will produce a type error, as that enum does not exist as a value anywhere in the code.

It's definitely quite shocking that a baseline TypeScript feature just doesn't work here for no obvious reason. This creates a mismatch between tooling and compilation, which is pretty bad considering the whole reason we're using TypeScript is for type safety. Would definitely like to see const enums fully supported.

@DorianGrey
Copy link
Collaborator

DorianGrey commented Sep 28, 2018

See my description above about the reason.
Even more details:
microsoft/TypeScript#16671
TypeStrong/ts-loader#331

Still thinking about a good way to disable transpileOnly without doing duplicate work via the type checker, and without losing its linting capabilites (there is no immediate replacement for that atm.).

@evaera
Copy link

evaera commented Sep 28, 2018

Yeah, I understand the technical reason, I just mean that there is no obvious reason as to why it doesn't work from a UX standpoint when debugging and running into this problem, especially because it results in a type error.

@ptommasi
Copy link

I had a similar issue (an enum inside a declared but not exported namespace was undefined once transpiled), I asked on stackoverflow and the const enum would have been my preferred option.

In my case, I exported the namespace (rather than declaring it) and exported the enum inside, as expected the generated code didn't have the inline value but at least the variable wasn't undefined anymore and the UI is working.

As a temporary fix, I exported the namespace with all the declaration (enum included) and I have to manually import it in most of my modules. That import line in every file is a bit annoying, but at least it's not breaking... Don't know if it helps someone.

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

No branches or pull requests

4 participants