-
Notifications
You must be signed in to change notification settings - Fork 4
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
better tree-shaking... compile ts to ESM (before bundling) #1044
Conversation
Netlify Deployments: |
Lighthouse results: results for https://ocw-hugo-themes-www-pr-1044--ocw-next.netlify.app/:
results for https://ocw-hugo-themes-www-pr-1044--ocw-next.netlify.app/search/:
results for https://ocw-hugo-themes-course-v2-pr-1044--ocw-next.netlify.app/:
|
If you're interested and want to see this in action with our config (but with something more readable than our actual bundles), you can check out https://github.com/mitodl/ocw-hugo-themes/tree/cc/ts-esm-experiment. That branch defines three new files (just for testing): // example_dog.js
export const dog = 'dog'
export const woof = (n: number) => 'woof'.repeat(n)
// example_cat.js
export const cat = 'cat'
export const meow = (n: number) => 'meow'.repeat(n)
// example_entry.js imports from cat and dog
// but does not use all of cat
import { cat } from "./example_cat"
import { dog, woof } from "./example_dog"
console.log(cat, "...?")
console.log(dog, woof(3)) Note that the
|
tldr: Changing `module` config changes the syntax used to import/export in the compiled code. The new value, "ES6", means that the code compiled by TS will include import/export statements. That code is then further compiled by Webpack, which removes the imports/exports during concatenation. Using ES6 import/exports in the typescript output improves Webpack's tree-shaking. (Or really...enables it. I believe webpack does not treeshake unless the input uses ES6 import/export syntax.) Related: TS has several sort of settings related to the syntax used in output: - `lib`: determines the default/builtin type definitions that are included when typechecking your code. - `target`: Spcecies environment in which your code is going to run, and controls how Typescript downlevels your code. > "The `target` setting changes which JS features are downleveled and which are left intact. For example, an arrow function `() => this` will be turned into an equivalent function expression if target is ES5 or lower." `target` determines the default value of `lib`. If `target` is ES5, you probably want `lib` to be ES5, too, unless you're doing further transpilation with another tool (like babel). - `module`: Determines the syntax used for exports. - `moduleResolution` determines how module identifiers ("some/thing" in like `import x from "some/thing"`) are interpretted.
8380058
to
462479e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Pre-Flight checklist
What are the relevant tickets?
Related to #1033, #1034
What's this PR do?
This PR changes the way we compile TS before being bundled by webpack: In our tsconfig, we had
module
set to CommonJS, whereas it should be set to ES6 As the Webpack Typescript docs say,How should this be tested
There should be no noticeable changes, other than that the output of
yarn build:webpack
should be a bit smaller.You could...
Run
yarn build:wepback
. It should succeed and the js file sizes (ls -lh base-theme/dist/js
) should be a bit smaller than they are onmain
branch.Run a site in production mode on this branch, e.g., by
yarn build abs/path/to/9.40-spring-2018 abs/path/to/ocw-hugo-projects/ocw-course-v2/config.yaml
npx serve path/to/9.40-spring-2018/dist
.serve
displayed in your terminal and check that the soruce code is viewable:Again, there should be no noticeable changes.
Background Info
The "module" config setting tells Typescript what module syntax to use when compiling TS files into JS files. Specifying ES6 means use ES6
import
/export
. Note: This ES6 import syntax does not end up in the final bundles. Again, from Webpack docs](https://webpack.js.org/concepts/manifest/):The advantage of compiling TS to JS is better tree-shaking (removal of unused code). It's not a huge difference at the moment:
but it's free, and I expect in the future it could matter more. (For example: I came across this issue while working on #1037, which I have decided to set aside. While working on #1037 , I found that because of changes in how Sentry organizes their code in v7, we Sentry v7's footprint on our bundle would be 54kb instead of 20kb without this config change.)