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

feat: Increase control over external collections #3155

Closed
3 tasks done
George-Payne opened this issue Nov 26, 2021 · 7 comments
Closed
3 tasks done

feat: Increase control over external collections #3155

George-Payne opened this issue Nov 26, 2021 · 7 comments
Labels
Feature: Want this? Upvote it! This PR or Issue may be a great consideration for a future idea.

Comments

@George-Payne
Copy link
Contributor

George-Payne commented Nov 26, 2021

Prerequisites

Describe the Feature Request

Currently, when importing a dist library into another stencil project, stencil will use the collection output to bake in the included components. Currently, the only control you have over this is excludeUnusedDependencies to only include used components, rather than every component in the collection.

It would be useful to be able to have more manual control over how this is handled, especially when working in a micro-frontend style environment.

Describe the Use Case

When working with the following scenario:

Host:

built with www output target

  • shell-app (depends on dist-one)
    • <shell-comp-a />

Design System:

built with dist output target

  • dist-one
    • <one-comp-a />
    • <one-comp-b />

Micro Frontends:

built with dist output target, with excludeUnusedDependencies

  • micro-a (depends on dist-one)
    • <ma-comp-a />

Situation 1

When all are compiled, we end up with:

  • shell-app

    • <shell-comp-a />
    • <one-comp-a />
    • <one-comp-b />
  • dist-one

    • <one-comp-a />
    • <one-comp-b />
  • micro-a

    • <ma-comp-a />
    • <ma-comp-b />
    • <one-comp-a />

Inside shell-app, micro-a and other microfrontends are (optionally) loaded at runtime via esm loader script.

  • shell-app loads.
    • <shell-comp-a />, <one-comp-a />, <one-comp-b /> are defined in customElements.
  • micro-a is loaded via the loader script
    • <ma-comp-a /> is defined in customElements.
    • <one-comp-b /> is already defined, so skipped

Currently, this is desired behaviour. Despite components being built and bundled multiple times, they are only loaded into the browser once.

Situation 2:

However, we have a breaking change in one-comp-b, and we increment the version of dist-one. Unfortunately, micro-a lags behind in development for whatever reason. When all built we end up with.

  • shell-app

    • <shell-comp-a />
    • <one-comp-a /> (v2.0.0)
    • <one-comp-b /> (v2.0.0)
  • micro-a

    • <ma-comp-a />
    • <ma-comp-b />
    • <one-comp-a /> (v1.0.0)

Once again,

  • shell-app loads.
    • <shell-comp-a />, <one-comp-a />, <one-comp-b /> are defined in customElements.
  • micro-a is loaded via the loader script
    • <ma-comp-a /> is defined in customElements.
    • <one-comp-b /> is already defined, so skipped

However, this time, when <mb-comp-a /> renders, it is expecting <one-comp-b /> at v1.0.0, so breaks.

Describe Preferred Solution

By providing more control over collections, we can avoid this situation in a variety of ways.

tagNameTransform

Much like with extras.tagNameTransform, that allows renaming components when using loader, we can change the name of the tag when loaded. However, as we have access to much more information at build time, we can also rename the tag at usage site, as well as provide some automatic transforms:

type TagNameTransformer = (tag: string, collection: string) => string;
type AutoTagNameTransform = "major" | "minor" | "patch";

interface Config {
  collections: {
    tagNameTransform:
      | false
      | TagNameTransformer
      | AutoTagNameTransform
      | Record<string, false | TagNameTransformer | AutoTagNameTransform>;
  };
}

export const config: Config = {
  namespace: "micro-a",

  collections: {
    // rename all tags to be postfixed with major version
    // <one-comp-a> -> one-comp-a-v1
    tagNameTransform: "major",

    // rename all tags to be postfixed with major and minor version
    // <one-comp-a> -> one-comp-a-v1-0
    tagNameTransform: "minor",

    // rename all tags to be postfixed with major, minor and patch version
    // <one-comp-a> -> one-comp-a-v1-0-0
    tagNameTransform: "patch",

    // Rename tags per collection
    // `dist-one` to be postfixed with major version, micro-a (itself) will be left alone
    tagNameTransform: {
      "micro-a": false,
      "dist-one": "major",
    },

    // Provide a custom rename, for all collections
    // <one-comp> -> ma-one-comp
    tagNameTransform: (tag) => `ma-${tag}`,

    // Provide a custom rename per collection
    tagNameTransform: {
      "dist-one": (tag) => `ma-${tag}`,
    },
  },
};

If we apply this to our previous situations, assuming that collections.tagNameTransform is set to "major".

Situation 1

As before, everything works as expected:

  • shell-app loads.
    • <shell-comp-a-v1 />, <one-comp-a-v1 />, <one-comp-b-v1 /> are defined in customElements.
  • micro-a is loaded via the loader script
    • <ma-comp-a-v1 /> is defined in customElements.
    • <one-comp-b-v1 /> is already defined, so skipped

Components continue to be built and bundled multiple times, and they are only loaded into the browser once.

Situation 2

This time, as the components are postfixed with their version, things work better:

  • shell-app loads.
    • <shell-comp-a-v2 />, <one-comp-a-v2 />, <one-comp-b-v2 /> are defined in customElements.
  • micro-a is loaded via the loader script
    • <ma-comp-a-v1 /> is defined in customElements.
    • <one-comp-b-v1 /> is not already defined, so is defined in customElements.

Now, component bundles can bring their own components at the expected version.

Mark collection(s) as external, handing over all control to the user.

interface Config {
  collections: {
    external: boolean | Record<string, boolean>;
  };
}

export const config: Config = {
  namespace: "micro-a",

  collections: {
    // all collections are eternal
    external: true,

    // only dist-one is external
    external: {
      "dist-one": true,
    },
  },
};

If we apply this to our previous situations, assuming that externalCollections is set to true across all:

  • shell-app

    • <shell-comp-a />
  • micro-a

    • <ma-comp-a />
    • <ma-comp-b />

The user can now handle component loading as they see fit.

Describe Alternatives

No response

Related Code

No response

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Nov 26, 2021
@splitinfinities
Copy link
Contributor

splitinfinities commented Nov 30, 2021

Hey! This can get super complex. Fortunately I understand the problem thanks to how well you covered the story here. Great work!

I think generally I agree with the idea of giving folks an escape hatch around how they bundle dependencies together in their component libraries, as the behavior today could result in becoming a footgun at scale. Enabling this as an opt-in feature can be beneficial for folks who want to provide libraries where their dependencies aren't bundled together, but are instead marked as peer dependencies, in an effort to help folks manage their versions without the magic of Stencil's compiler combining all Stencil libraries together during the compiler process.

One crucial thing I think the Stencil team will need in order to come back and really "grok" this is to create a repo covering the current behavior - check in the dist directory too so that you can point to where the other collections are included in those files to really "paint" the pain you're describing.

If you happen to take a crack at the code, create another branch on that repo that can then be compared to the new result that allows those Stencil libraries to be opted out.

I feel like one roadbump you may discover is how Typings work when you're deciding to opt-out of bundling the component libraries together. This may be solved by blocking compilation of a Stencil library until all external dependencies are added and available with some user-friendly message... But I'm currently unsure of any other hard edges that may end up emerging from this new behavior around external typings.

We're excited to see what you put together! Let me know how I can help!

@johnjenkins
Copy link
Contributor

johnjenkins commented Dec 1, 2021

Another need / use-case I have for this feat...
In a PWA I have ionic as a nice-to-have to fill in any gaps my design system might have - import @ionic/core. In reality I will use 3-4 components, but in my final generated manifest I have the whole ionic lib - there is no way (I don't think) to strip out / ignore certain components because they're all just p-randomstring.js.
It results in my manifest being much bigger than I would want.

*edit - @George-Payne pointed out that I can exclude all unused deps by using config opt excludeUnusedDependencies - however perhaps that could be added specifically to external collections too

@George-Payne
Copy link
Contributor Author

George-Payne commented Dec 1, 2021

but in my final generated manifest I have the whole ionic lib - (I don't think) there is no way to strip out / ignore certain components because they're all just p-randomstring.js.

This is one of the current usecases for excludeUnusedDependencies.

It's possible that you would want to control this on a per collection basis, so it may make sense to move it into the collections section of the config.

interface Config {
  collections: {
    excludeUnused: boolean | Record<string, boolean>;
  };
}

@johnjenkins
Copy link
Contributor

johnjenkins commented Dec 1, 2021

ok - just found a place where excludeUnusedDependencies as-is doesn't serve my requirements :)
My design system has a few helper functions that create some components imperatively (not declaratively) - tooltip, alert, toast, dialog - these are therefore not picked up as used so are not included

@splitinfinities
Copy link
Contributor

I asked a question over on #3138 - how do these two collaborate together in folks eyes? Feels related and may benefit from thinking about these together. I may be off base as well! Thoughts?

@George-Payne
Copy link
Contributor Author

George-Payne commented Dec 2, 2021

Feels related and may benefit from thinking about these together.

Yes, I'm not sure how I missed that issue when creating this one. They are both trying to solve basically the same issue, and mostly propose a similar solution, at least in tagNameTransform. (exclude is more intended as an escape hatch).

#3138 is maybe more general, as this was only intended to be used in stencil to stencil situations, however, tagNameTransform could also be applied to base collection (the components defined in the project being built) rather than just the consumed components as is proposed here. I can see how this could be especially useful when generating code for other frameworks (as mentioned in #3138 (comment))

We could possibly merge the issues, or consider this as a proposal of how to fix #3138 (with some tweaks)

@splitinfinities splitinfinities added the Feature: Want this? Upvote it! This PR or Issue may be a great consideration for a future idea. label Dec 2, 2021
@ionitron-bot ionitron-bot bot removed the triage label Dec 2, 2021
@George-Payne
Copy link
Contributor Author

I had some time to look at this a bit more today, and I've come to the decision that this proposed solution to versioning is overcomplicated and has too many issues needing to be baked into the solution (needing to also transform the tag names in CSS, provide a way to get a tag name when creating elements via document.createElement, etc).

I think ultimately this will be solved by Scoped Custom Element Registries, and if a versioning solution is built into stencil, it would probably be better off being a polyfill of that. But this would be better discussed in #3138.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Want this? Upvote it! This PR or Issue may be a great consideration for a future idea.
Projects
None yet
Development

No branches or pull requests

3 participants