Lion with Angular, Unpkg (optional chaining)

[MendyBerger]

Expected behavior

Importing and using lion in Angular should work

Actual Behavior

When importing @lion/select-rich I'm getting the following error:

node_modules/@lion/select-rich/src/LionSelectInvoker.d.ts:8:14 - error TS2417: Class static side 'typeof LionSelectInvoker' incorrectly extends base class static side 'typeof LionButton'.
  Types of property 'styles' are incompatible.
    Type '(CSSResult | CSSResult[])[]' is not assignable to type 'CSSResult[]'.
      Type 'CSSResult | CSSResult[]' is not assignable to type 'CSSResult'.
        Type 'CSSResult[]' is missing the following properties from type 'CSSResult': cssText, styleSheet

Additional context

Which component and version of it are you using:
@lion/[email protected]

Thanks in advance, I really like the idea of a style'less component library and I'm looking forward to using it!

[jorenbroekema]

In your TS Config you can put "skipLibCheck": true as a workaround, this will silence errors if they come from types in your node_modules.

You might need this as we have some other @ts-expect-error / @ts-ignore statements in our source code that we haven't been able to work around yet.. so the chances you will get other errors from lion's type definition files is somewhat substantial. Alternatively, just bug us everytime you find a problem, then we know which type issues we need to fix with urgency and which ones are not a problem for our users.

I'll list it as a bug and try to fix it asap.. Lit-element's styles getter is always a little awkward because it accepts an array of CSSResult, or a single CSSResult, and it flattens if you nested arrays of CSSResults... @daKmoR were you still planning on proposing that Lit-element always accepts only an array to simplify things?

[MendyBerger]

@jorenbroekema thanks for the idea, I added skipLibCheck and I'm no longer getting the previous error.

However, I'm now getting a new error:

Error: ./node_modules/@lion/overlays/src/utils/inert-siblings.js 11:78
Module parse failed: Unexpected token (11:78)
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
|  */
| export function setSiblingsInert(element) {
>   const parentChildren = /** @type {HTMLCollection} */ (element.parentElement?.children);
|   for (let i = 0; i < parentChildren.length; i += 1) {
|     const sibling = parentChildren[i];

I think that webpack 4 can't handle optional chaining, but I'm not sure

[jorenbroekema]

I will release Lion pretty soon (within the hour), you can check without skipLibCheck if you want then to see if it's actually resolved.

For optional chaining support you need webpack 5 minimum indeed :)

[MendyBerger]

@jorenbroekema Would transpiling out optional chaining on your side be an option? Don't think that webpack 5 with Angular is all worked out yet

Alternatively, importing lion from a cdn with a script tag can be an option for Angular devs

[MathieuPuech]

the same issue happen on @open-wc/scoped-elements package, you can see configurations needed to avoid this with webpack 4 in this issue: open-wc/open-wc#1979 (comment)

[MathieuPuech]

webpack 5 issue for Angular is here: angular/angular-cli#17556

[MendyBerger]

Looks like unpkg.com is unhappy with optional chaining as well, when getting https://unpkg.com/@lion/[email protected]/index.js?module I'm getting the following response:

Cannot generate module for @lion/[email protected]/src/OverlayMixin.js

SyntaxError: unknown: Support for the experimental syntax 'optionalChaining' isn't currently enabled (93:43):

  91 |           ...(this.config.popperConfig || {}),
  92 |           modifiers: [
> 93 |             ...(overlayConfig.popperConfig?.modifiers || []),
     |                                           ^
  94 |             ...(this.config.popperConfig?.modifiers || []),
  95 |           ],
  96 |         },

Add @babel/plugin-proposal-optional-chaining (https://git.io/vb4Sk) to the 'plugins' section of your Babel config to enable transformation.

undefined

[jorenbroekema]

We have refactored code in the past by removing syntax that makes unpkg happy, so I'm willing to accept a pull request refactoring at least the code in Lion's src to not use optional chaining. We also use it inside tests where it's very convenient to have it, since we don't publish those I guess we can keep optional chaining inside test files.

[tlouisse]

Wouldn't it be possible to compile out optional chaining during the prepublish step?

[jorenbroekema]

Also fine with me, but we don't use optional chaining that much, and it doesn't save much code anyway (a line or two), so I'm down with either just removing it from our src or transpiling it out on prepublish.

[kishok]

Is optional chaining transpile to target support for Lion Packages released ? I am using @lion/dialog in my component, still facing the above issue in angular v11.x (webpack 4) and tsconfig target : "es6"

[jorenbroekema]

Ah thanks for the bump! Creating a fix now

[jorenbroekema]

Hmff, the prepublish idea is not gonna work.

Babel transpiling the optional chaining causes a ton of typescript errors (440 or so) because it adds newlines everywhere and just really messes with the formatting.

Running a prettier format does not resolve this either..

I also tried https://github.com/conartist6/babel-plugin-recast to prevent babel from messing up the code, and it seems to help, however, it makes the optional chaining plugin not behave properly, some of the optional chaining does not get transpiled out. Even if it did work, still a bunch of typescript errors (16 or so) because typescript is not smart enough to infer that a guard like this:

if ((_this$config22 = this.config) !== null && _this$config22 !== void 0 && _this$config22.contentWrapperNode && 
  this.placementMode === 'local') {
  /** config [l2],[l3],[l4] */
  this.__contentWrapperNode = this.config.contentWrapperNode;
}

is not sufficient to infer that config is not undefined, because we check this.config !== null "indirectly" by first assigning it to _this$config22

So essentially the only good optional chaining plugin for babel that I can find is not typescript-friendly even with extra plugins to prevent babel from adding newlines everywhere.

The only way forward I see is that tools like bundlephobia and unpkg update their code to support optional chaining. I believe LitElement v3 is planning to ship optional chaining (it makes their bundlesize way smaller), and I expect they will put some pressure on these tools to update as well. Here's one PR unpkg/unpkg#282, perhaps we can all upvote that one.

We can also change our source code to not use optional chaining, this is a list of packages where we use it in src folder:
@lion/accordion
@lion/calendar
@lion/collapsible
@lion/combobox
@lion/form-core
@lion/helpers
@lion/input-amount
@lion/input-iban
@lion/localize
@lion/overlays
@lion/steps

Pull request contributions are welcome to remove the optional chaining in those packages, it will be a while before this gets enough priority that we do that ourselves.

[MartinKolarik]

Hi, Martin from jsDelivr here. We recently added a feature similar to unpkg's ?module and it does work with optional chaining, so maybe that will help and you won't have to refactor all the code: https://esm.run/@lion/[email protected]/src/OverlayMixin.js. Feel free to ping me if you run into any issues.

[jorenbroekema]

Ah hi! Sweet! I tried to use esm.run but I can't really seem to make it work: https://codepen.io/Falx/pen/dyOYRbw

It seems to load the dependency properly, the native button which lion-button wraps is spawned correctly, but its shadowDOM is not rendered properly. I face this issue with all lion components if I import them from esm.run. Any idea why this is? The rendering engine that is used by lion is lit-html, the only time I recall having it seen output [object Object] is when there are multiple installations of lit-html.

[MartinKolarik]

Someone already reported the issue with lit-html a few days back. Didn't have the time to track it down yet but expect I'll soon. I suspect it loads two copies somehow and then fails an instanceof check somewhere.

[jorenbroekema]

Could it be that in the dependency resolution lit-html isn't deduped like how NPM would dedupe? In lion only @lion/core depends on and imports from lit-html directly, the other @lion/<packages> depend on @lion/core with a fixed version to import lit-html stuff (@lion/core re-exports it), so it could be that @lion/core installs lit-html, but @lion/button also installs lit-html separately as a dependency of its dependency @lion/core instead of deduping it to the install of @lion/core

[MartinKolarik]

it could be that @lion/core installs lit-html, but @lion/button also installs lit-html separately as a dependency of its dependency @lion/core instead of deduping it to the install of @lion/core

They would indeed be resolved separately but the result would be the same URL (provided package.json has the same/similar required version for both).

Looking at the request log now, I see requests for both
https://cdn.jsdelivr.net/npm/[email protected]/+esm and https://cdn.jsdelivr.net/npm/[email protected]/lit-html.js/+esm so this is likely related to how import 'lit-html' vs import 'lit-html/lit-html.js' works (it's the same file but once specified implicitly and resolved via package.json and then specified explicitly in the URL).

Edit: this is likely the reason. Checking what we can do about it...

Edit 2: don't want to spam this thread so more details are at jsdelivr/jsdelivr#18263 (comment)

[jorenbroekema]

unpkg/unpkg#282 got merged just now. @MendyBerger can you check whether your issue with optional chaining in unpkg is resolved?

[MendyBerger]

Now hitting the same problem in a few other places

https://unpkg.com/@lion/[email protected]/src/FormControlMixin.js?module
https://unpkg.com/@lion/[email protected]/src/OverlayMixin.js?module
https://unpkg.com/@lion/[email protected]/src/OverlayController.js?module
https://unpkg.com/@lion/[email protected]/src/utils/inert-siblings.js?module

[jorenbroekema]

Hmmmm, how is it now? I think it was taking a while for the changes to be deployed to unpkg.com, it was only on dev.unpkg.com for a few days. Now if I click those links I don't get errors.

I would no longer recommend unpkg though to be honest, because it doesn't support export maps.

Right now, the right way to import lion-dialog element is like this:

import '@lion/dialog/define';

return html`
  <lion-dialog></lion-dialog>
`;

https://lion-web.netlify.app/blog/controlling-exports/

Most CDNs don't seem to support export maps / import maps, but if you want to use a CDN I would suggest using jspm, it allows you to generate importmap.

https://codepen.io/Falx/pen/BapwYgK?editors=1000. here is a nice example for importing lion-select-rich using import maps using JSPM CDN.

To generate import maps from Lion's export maps, I suggest using: https://generator.jspm.io/ , it also has a nice sandbox environment that you can fiddle in. It also comes with the shims needed for supporting other browsers than latest Chrome, as well as compatibility with SystemJS if that's what you're using.