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

Export maps "convention/recommendation" for web components #7

Open
daKmoR opened this issue Feb 17, 2021 · 16 comments
Open

Export maps "convention/recommendation" for web components #7

daKmoR opened this issue Feb 17, 2021 · 16 comments

Comments

@daKmoR
Copy link
Collaborator

daKmoR commented Feb 17, 2021

Exports maps are now available in node & rollup & webpack.

Maybe we can have a convention/recommendation for what to define for web components? 🤔

Given the following component/package my-input;

src/MyInput.js
src/my-input.js
index.js

From a users point of view it would probably be nice to have imports like this:

import { MyInput } from 'my-input'; // <--- side effect free
import 'my-input/element'; // <--- side effect customElements.define(...)

This would lead to the following export map

"name": "my-input",
"exports": {
  ".": "./index.js",  // <--- side effect free
  "./element": "./src/my-input.js"
}

What do you think?

@thepassle
Copy link

maybe:

import 'my-input/define'; // <--- side effect customElements.define(...)
// or
import 'my-input/defined'; // <--- side effect customElements.define(...)

So the actual side effect is made clear in the specifier?

@daKmoR
Copy link
Collaborator Author

daKmoR commented Feb 17, 2021

uh I like 🤗

import 'my-input/define';

@manolakis
Copy link

manolakis commented Feb 17, 2021

I like the define or maybe definition option for side effects because it allows us to export several components in just one package.

"name": "my-lib",
"exports": {
  ".": "./index.js",  // <--- side effect free
  "./first-element": "./src/first-element/FirstElement.js", // <-- side effect free
  "./first-element/definition": "./src/first-element/fist-element.js",
  "./second-element": "./src/second-element/SecondElement.js", // <-- side effect free
  "./second-element/definition": "./src/second-element/second-element.js"
}

@justinfagnani
Copy link
Collaborator

I think this is maybe useful to talk about once scoped custom element registries have landed, but I think it's actively bad to have non-self-defining element classes right now, and I personally would advocate for all elements to be defined in the same module as the class declaration. It's unsafe to do otherwise: https://justinfagnani.com/2019/11/01/how-to-publish-web-components-to-npm/#always-self-define-elements

@daKmoR
Copy link
Collaborator Author

daKmoR commented Feb 17, 2021

we had and we still have lots of issues with side effects (not only custom elements 😅) ... most of these issues arise because users do something like import { foo } from 'foo'; and assume it's side effect free... but it's not 😅
so for that reason, we try to keep side effects always out of the bare import...

and as customElements.define is a side effect we also keep it outside...

and as we are already making have use of a scoping mechanism (similarish to scoped custom element registries) it definitely makes sense for us...
and considering the path forward and the HUGE impact, it would have to change this later we opted to be a little more forward-thinking in this case.

I personally believe it's a general good recommendation (e.g. side effect free main entry point and have sepearte entry points for side effects)

@justinfagnani
Copy link
Collaborator

The thing is that non-self-registering elements are fundamentally unsafe. Any two consumers who want to use the element must agree on a tag name, or one will fail. The only party who can choose a canonical name is the element class itself. If one consumer imports the side-effect-free module and registers the element, it'll make every other consumer who imports the self-registering module fail.

@daKmoR
Copy link
Collaborator Author

daKmoR commented Feb 18, 2021

The only party who can choose a canonical name is the element class itself

this is not true for nested dependencies 😅. which will in such a case always result in a browser error you can not fix without forcing the same version to everyone (which might be impossible)

At ING we are rather lucky as we can "enforce" usage of scoped elements by not even exporting the customElements.define. e.g. we only export side effect free classes and we expect/force you to use it via ScopedElements
=> it allowed us to support 2 major versions of our component set at the same time

Comparison

So we basically have 2 options

  1. Recommend self-registering elements e.g. classes +customElements.define in the same file and always have side effects

    👍  will be a one to one mapping name - classes
    👎  main entry points will always have a side effects (or you don't export classes?)
    👎  will break if you want to use a scoped registry (or a "polyfill" for it) => also a huge effort to change it

  2. Recommend separating classes + customElements.define

    👍  side effect free main entry point
    👍  supports scoped registry (or a "polyfill" for it)
    👎  requires info/warning on how to use (what and when) - or even some form of "enforcement"

I'm in favor of 2 as I think it's more forward-looking and it mainly requires an info/warning while as 1 can result in situations where you basically can't use that element in your app. (e.g. in ING if you build a reusable component or feature that will be used in multiple apps then you can not use any self-registering elements as it will break as soon as 2 versions (via nesting) of that element get used)

@justinfagnani
Copy link
Collaborator

main entry points will always have a side effects

what are the problems with side-effects specifically?

will break if you want to use a scoped registry

what did you mean by this point? You can have a self-defining element and use it in a scoped registry.

@manolakis
Copy link

manolakis commented Feb 18, 2021

If we can't import a component without it's side effect means that we can't use different major versions of the same component at the same time because the browser will fail in the definition of the second loaded version.

@ConradSollitt
Copy link

Has anyone experienced issues from Web Component Side effects when using something like in a browser?

// Both calls on the same app
import { MyInput } from './my-input';
import './my-input';

I myself have never needed to do that and without testing I would expect ./my-input to be loaded only once so the side-effect from define() would not raise an issue. Just guessing though.

Personally I would prefer to keep simple mapping for Export Maps (one map = one script) regardless of side effects or not.

@daKmoR
Copy link
Collaborator Author

daKmoR commented Feb 19, 2021

if you don't have nested versions then indeed there is no need for any of that... so in the example above ./my-input will only be executed once

we however need to maintain multiple major versions of the same component in one app.

in short, if you somehow execute customElements.define via your index.js and you have this structure

node_modules/red-el/index.js <-- 1.x
node_modules/my-feat/index.js 
node_modules/my-feat/node_modules/red-el/index.js <-- 2.x
src/app.js

and you then import it like this

// src/app.js
import { RedEl } from 'red-el';
import { MyFeat } from 'my-feat';

it will lead to the error failed to execute 'define' on 'CustomElementRegistry': the name "red-el" has already been used with this registry. Why? because node_modules/red-el/index.js and node_modules/my-feat/node_modules/red-el/index.js are separate files.

Note: this also happens if you have nested versions for any other reason e.g. it could be hard version dependencies. e.g. 1.2.3 and 1.4.1 of red-el being used.

if index.js is side effect free (e.g. without customElements.define) then it works just fine

You can read more details in the Scoped Elements Motivation Section.

PS: You may ask why even bother? force a single version... this is imho a bad argument as it's really important for migration phases - if you have 1000 components you not gonna upgrade them in one go from 1.x to 2.x...
PPS: Additionally most other popular frameworks like (React, Angular, Vue...) support multiple major versions, therefore, developers are surprised if it doesn't work for web components... and then you get the typical Web Components aren't ready response - let's avoid that.

what are the problems with side-effects specifically?

let's park this topic for now - it's not 100% relevant for this discussion

@hmans
Copy link

hmans commented Feb 19, 2021

I've been following this discussion and learning a lot from it -- I was just about to do some refactoring of my library (three-elements) to allow for selectively importing individual tags and even customizing their prefixes, but will now rather strive to keep things simple and non-surprising.

I do have on question though. What is your take on loading/defining custom elements in a hot reloading dev server context? In three-elements, I've had to whip up my own helper function that wraps customElements.define and does a quick check if the element is already defined, because just calling it directly would make the library entirely unusable in development environments that don't perform full page reloads when code is changed.

(I'm aware that this also means that updated versions of my classes are not registered; but I'm not talking about the development of the library itself, but of projects that use it.)

It feels to me that, unless I've missed something obvious (and I often do), the immediate use of customElements.define is outright problematic (unless we're all agreeing that the use of hot-reloading development environments is problematic, which some people do in fact argue.)

Also please consider that "hot reloading" is not always just "hot module reloading" from React et al. This problem occurs whenever new code is pushed to the browser without it doing a full page reload. (For example: any Create-React-App project, or any Codesandbox sandbox.)

@daKmoR
Copy link
Collaborator Author

daKmoR commented Feb 19, 2021

Indeed HMR requires special handling (in any framework/system)... for web components there is no difference... the special handling, in this case, is all about never reregistering the component again but by "patching/proxying" function calls to the latest implementations.

You can read some more details here https://open-wc.org/docs/development/hot-module-replacement/#how-it-works

@ConradSollitt
Copy link

Thanks for your detailed response @daKmoR

@hmans
Copy link

hmans commented Feb 19, 2021

Indeed HMR requires special handling (in any framework/system)... for web components there is no difference... the special handling, in this case, is all about never reregistering the component again but by "patching/proxying" function calls to the latest implementations.

Thanks for the link! In the context of the discussion in this issue, I wonder if this maybe should be part of the recommendation? I'm just -- once again, naively, I'm still relatively new to all this -- guessing that if the "official" recommendation is to always define from an export, there's a high risk of creating surprising/blocking behavior around hot reloading. (This is the main reason why I was almost going to refactor my library to require the user to explicitly invoke a function that would perform the defining.)

@ConradSollitt
Copy link

Drive-by comment - After reading @daKmoR comment regarding my question for the need; at a high level this reminds me of sites that have the need for multiple jQuery versions due to different plugins.

I have created a few production apps at work with Web Components now (and some open source ones on Github) but luckily I haven't had to work about component version yet.

I'm about ready to go to bed though so need to follow up and read all the links in more detail tomorrow and again next week 🤔

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

No branches or pull requests

6 participants