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

fix(compiler): removed support for tagNameTransform #5853

Closed
wants to merge 2 commits into from

Conversation

christian-bromann
Copy link
Member

What is the current behavior?

It seems like Stencil currently allows to transform the tag name at runtime. The feature was introduced in https://github.com/ionic-team/stencil/pull/2343/files but was never really introduced nor added to the docs.

What is the new behavior?

I suggest to remove this entirely since we don't have any tests for this nor do we know how it suppose to behave.

Documentation

Does this introduce a breaking change?

  • Yes
  • No

Technically yes but since we never documented it, it was never actually been released as a feature.

Testing

Other information

n/a

@christian-bromann christian-bromann requested a review from a team as a code owner June 24, 2024 19:56
Copy link
Contributor

github-actions bot commented Jun 24, 2024

PR built and packed!

Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/9651629167/artifacts/1633239046

If your browser saves files to ~/Downloads you can install it like so:

unzip -d ~/Downloads ~/Downloads/stencil-core-4.18.3-dev.1719259258.093e4de.tgz.zip && npm install ~/Downloads/stencil-core-4.18.3-dev.1719259258.093e4de.tgz

Copy link
Contributor

github-actions bot commented Jun 24, 2024

@stencil/core@4.19.2 ts
tsc --noEmit --project scripts/tsconfig.json && tsx scripts/tech-debt-burndown-report.ts

--strictNullChecks error report

Typechecking with --strictNullChecks resulted in 1068 errors on this branch.

That's the same number of errors on main, so at least we're not creating new ones!

reports and statistics

Our most error-prone files
Path Error Count
src/dev-server/index.ts 37
src/dev-server/server-process.ts 32
src/compiler/prerender/prerender-main.ts 22
src/runtime/vdom/vdom-render.ts 22
src/runtime/client-hydrate.ts 20
src/runtime/vdom/test/patch.spec.ts 19
src/runtime/vdom/test/util.spec.ts 19
src/screenshot/connector-base.ts 19
src/testing/puppeteer/puppeteer-element.ts 19
src/dev-server/request-handler.ts 15
src/compiler/prerender/prerender-optimize.ts 14
src/compiler/sys/stencil-sys.ts 14
src/runtime/connected-callback.ts 14
src/sys/node/node-sys.ts 14
src/compiler/prerender/prerender-queue.ts 13
src/compiler/sys/in-memory-fs.ts 13
src/runtime/set-value.ts 13
src/compiler/output-targets/output-www.ts 12
src/compiler/transformers/test/parse-vdom.spec.ts 12
src/compiler/transformers/transform-utils.ts 12
Our most common errors
Typescript Error Code Count
TS2322 338
TS2345 326
TS18048 188
TS18047 99
TS2722 27
TS2531 19
TS2532 18
TS2790 11
TS2454 10
TS2352 9
TS2769 8
TS2416 7
TS2538 4
TS18046 2
TS2684 1
TS2430 1

Unused exports report

There are 15 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!

Unused exports
File Line Identifier
src/runtime/bootstrap-lazy.ts 21 setNonce
src/screenshot/screenshot-fs.ts 18 readScreenshotData
src/testing/testing-utils.ts 198 withSilentWarn
src/utils/index.ts 145 CUSTOM
src/utils/index.ts 245 NODE_TYPES
src/utils/index.ts 269 normalize
src/utils/index.ts 7 escapeRegExpSpecialCharacters
src/compiler/app-core/app-data.ts 25 BUILD
src/compiler/app-core/app-data.ts 115 Env
src/compiler/app-core/app-data.ts 117 NAMESPACE
src/compiler/fs-watch/fs-watch-rebuild.ts 123 updateCacheFromRebuild
src/compiler/types/validate-primary-package-output-target.ts 82 satisfies
src/compiler/types/validate-primary-package-output-target.ts 82 Record
src/testing/puppeteer/puppeteer-declarations.ts 496 WaitForEventOptions
src/compiler/sys/fetch/write-fetch-success.ts 7 writeFetchSuccessSync

@johnjenkins
Copy link
Contributor

Hi!
just to make it known this feature is definitely used by the community https://dev.to/sanderand/running-multiple-versions-of-a-stencil-design-system-without-conflicts-2f46 :)

@christian-bromann christian-bromann added the Stencil v5 This is slated for Stencil v5 (Release date TBD) label Jun 25, 2024
@TRMW
Copy link

TRMW commented Jul 3, 2024

Also wanted to chime in and say my team is currently using this feature!

@christian-bromann
Copy link
Member Author

@johnjenkins @TRMW thanks for providing feedback. Mind sharing in what context you are using this? What is the problem you are trying to solve?

@johnjenkins
Copy link
Contributor

johnjenkins commented Jul 3, 2024

I believe, essentially, the use cases are outlined by scoped custom element registries so if stencil can work with that proposal / polyfill then that’d be an alternative / better way forward

@ryuran
Copy link

ryuran commented Jul 9, 2024

I confirm, we also use it and struggle with angular react output target.

@TRMW
Copy link

TRMW commented Jul 9, 2024

We are also using it as an alternative to scoped custom element registries. We use Stencil for a design system component library, and there are sometimes times when multiple versions of that library end up on the same page. We use this to allow teams to prefix/suffix their custom element names to ensure they always get the version they expect.

@christian-bromann christian-bromann force-pushed the cb/remove-transformTagName branch from 087e4ed to e713520 Compare July 26, 2024 18:20
@tanner-reits tanner-reits removed their request for review August 9, 2024 16:00
@christian-bromann
Copy link
Member Author

Closing as we plan to use this feature to support scoped components.

@christian-bromann christian-bromann deleted the cb/remove-transformTagName branch November 20, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stencil v5 This is slated for Stencil v5 (Release date TBD)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants