lit / lit.dev

The Lit website
https://lit.dev
BSD 3-Clause "New" or "Revised" License
117 stars 179 forks source link

[docs] v3.0 bump from es2019 to es2021 requires tooling changes #1195

Closed gavinbarron closed 1 year ago

gavinbarron commented 1 year ago

Which package(s) are affected?

Lit Core (lit / lit-html / lit-element / reactive-element)

Description

When testing an upgrade to lit@3.0.0-pre.0 for MGT I encountered an interesting issue.

We have a monorepo with components built on lit and a some dependent packages. One of our dependent packages uses webpack as a bundler. After installing the preview package and trying to build the webpack based package failed to build with errors like this:

[20:17:17] Error - [webpack] 'dist':
/home/gavinb/dev/mgt/node_modules/lit/node_modules/lit-html/directives/class-map.js 6:106
Module parse failed: Unexpected token (6:106)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|  * Copyright 2018 Google LLC
|  * SPDX-License-Identifier: BSD-3-Clause
>  */const e=s(class extends i{constructor(t){if(super(t),t.type!==r.ATTRIBUTE||"class"!==t.name||t.strings?.length>2)throw Error("`classMap()` can only be used in the `class` attribute and must be the only part in the attribute.")}render(t){return" "+Object.keys(t).filter((s=>t[s])).join(" ")+" "}update(s,[i]){if(void 0===this.it){this.it=new Set,void 0!==s.strings&&(this.st=new Set(s.strings.join(" ").split(/\s/).filter((t=>""!==t))));for(const t in i)i[t]&&!this.st?.has(t)&&this.it.add(t);return this.render(i)}const r=s.element.classList;for(const t of this.it)t in i||(r.remove(t),this.it.delete(t));for(const t in i){const s=!!i[t];s===this.it.has(t)||this.st?.has(t)||(s?(r.add(t),this.it.add(t)):(r.remove(t),this.it.delete(t)))}return t}});export{e as classMap};
| //# sourceMappingURL=class-map.js.map
|
 @ /home/gavinb/dev/mgt/node_modules/lit/directives/class-map.js 1:0-46 1:0-46
 @ ../mgt-components/dist/es6/components/mgt-agenda/mgt-agenda.js
 @ ../mgt-components/dist/es6/components/components.js
 @ ../mgt-components/dist/es6/index.js
 @ ./lib/index.js

This is because of this change: https://github.com/lit/lit/commit/85ca81a7e935e554b2f40303b7956b638edb3176

The move from es2019 to es2020 introduces optional chaining and nullish coalescing operators which the existing webpack config does not know how to process. I'm reasonably confident that I can update the webpack config to handle this change, but it might be useful to call this change out in the upgrading documentation.

Reproduction

git clone https://github.com/microsoftgraph/microsoft-graph-toolkit.git mgt cd mgt git checkout chore/lit-upgrade yarn && yarn build

Observe the errors when building the mgt-spfx package

Workaround

I'm working on a fix, to add new loaders into the webpack config to handle the es2021 code

Is this a regression?

Yes. This used to work, but now it doesn't.

Affected versions

failing in 3.0.0-pre.0 working in 2.7.5

Browser/OS/Node environment

Browser: N/A OS: Ubuntu 20.04 node: 16.19.1 yarn: 1.22.19 npm: 6.14.17 (although we don't use npm on this project)

augustjk commented 1 year ago

Thanks for the report!

This is a good callout to mention in the upgrade guide that existing project build tools might also need updating or additional configuring to handle newer syntax.

cc: @AndrewJakubowicz

augustjk commented 1 year ago

With regards to webpack, it looks like webpack v5 should have support for optional chaining without loaders. https://github.com/webpack/webpack/releases/tag/v5.0.0-beta.25

This might need updating on @microsoft/sp-build-web depended by the mgt-spfx package which appears to be using webpack v4. https://unpkg.com/browse/@microsoft/sp-build-web@1.17.3/package.json

gavinbarron commented 1 year ago

Yeah it's 100% the SPFx build chain.

On Wed, Jun 14, 2023, 4:43 PM Augustine Kim @.***> wrote:

With regards to webpack, it looks like webpack v5 should have support for optional chaining without loaders. https://github.com/webpack/webpack/releases/tag/v5.0.0-beta.25

This might need updating on @microsoft/sp-build-web depended by the mgt-spfx package which appears to be using webpack v4. @.**@./package.json

— Reply to this email directly, view it on GitHub https://github.com/lit/lit.dev/issues/1195, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWK6HF6YOCBUEMVE7UUNZTXLJEBFANCNFSM6AAAAAAZG53IAU . You are receiving this because you authored the thread.Message ID: @.***>

rictic commented 1 year ago

Taking this as a documentation issue

AndrewJakubowicz commented 1 year ago

I confirmed that this is not a blocker for Webpack 4 users. They will need to use babel to downlevel the ES2021 JavaScript in the Lit 3.0 packages so the Webpack 4 parser can parse them without crashing. Webpack 4's internal parser crashes on nullish coallescing (??=), or optional chaining (?.).

See this stackblitz for a minimal demo that shows how to transpile the lit node_modules in a Webpack 4 project: https://stackblitz.com/edit/node-tp9xrf?file=README.md

Note, this is only needed for Webpack 4, as the Webpack 5 internal parser can handle the Lit packages without needing the babel rule.

gavinbarron commented 1 year ago

I got it working in our build chain with this change: https://github.com/microsoftgraph/microsoft-graph-toolkit/pull/2759/commits/773dd7ccebbe36346ec9a7d5b0397749f53d2ac1#diff-ff64d7a7016a9277adb37034a95a521c52ef14d238603f44d7a29d12cf993008