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

bug: Removing polyfills except for es5 builds breaks existing installs #5780

Open
3 tasks done
gavmck opened this issue May 20, 2024 · 17 comments
Open
3 tasks done

bug: Removing polyfills except for es5 builds breaks existing installs #5780

gavmck opened this issue May 20, 2024 · 17 comments
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil Stencil v5 This is slated for Stencil v5 (Release date TBD)

Comments

@gavmck
Copy link

gavmck commented May 20, 2024

Prerequisites

Stencil Version

4.18.1

Current Behavior

The PR to remove polyfills unless explicitly building es5 means that, although addPolyfills is still defined as being exported in the types, the function does not exist. This meant when we our package lock updated from 4.14.x to 4.18.1, the install broke.

PR #5725

Expected Behavior

Might be cleaner to keep exporting the function, but make it do nothing so the import doesn't explode before removing in the next major.

System Info

No response

Steps to Reproduce

Install stencil 4.14.x and use applyPolyfills.
Update to stencil 4.18.1, watch build explode.

Code Reproduction URL

https://sorry.dont/have/this.biz

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label May 20, 2024
@christian-bromann
Copy link
Member

@gavmck thanks for raising the issue.

Unfortunately I was not able to verify that exploding build 😉 I created a new Stencil project, build it with v4.14.0 and then again with v4.18.1 and everything was fine:

❯ npm i
npm WARN deprecated puppeteer@21.11.0: < 22.5.0 is no longer supported

added 349 packages, and audited 350 packages in 6s

38 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
❯ npm run build

> issue5780@0.0.1 build
> stencil build

[18:02.0]  @stencil/core
[18:02.1]  v4.14.0 🚡
[18:02.8]  build, issue5780, prod mode, started ...
[18:02.8]  transpile started ...
[18:04.0]  transpile finished in 1.20 s
[18:04.0]  copy started ...
[18:04.0]  generate custom elements + source maps started ...
[18:04.0]  generate lazy + source maps started ...
[18:04.1]  copy finished (0 files) in 107 ms
[18:04.2]  generate custom elements + source maps finished in 213 ms
[18:04.3]  generate lazy + source maps finished in 299 ms
[18:04.3]  build finished in 1.55 s

❯ rm -r node_modules/ package-lock.json
❯ npm i
npm WARN deprecated puppeteer@21.11.0: < 22.5.0 is no longer supported

added 349 packages, and audited 350 packages in 2s

38 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
❯ npm run build

> issue5780@0.0.1 build
> stencil build

[18:22.1]  @stencil/core
[18:22.3]  v4.18.1 🏍
[18:22.8]  build, issue5780, prod mode, started ...
[18:22.9]  transpile started ...
[18:24.0]  transpile finished in 1.19 s
[18:24.0]  copy started ...
[18:24.1]  generate custom elements + source maps started ...
[18:24.1]  generate lazy + source maps started ...
[18:24.1]  copy finished (0 files) in 104 ms
[18:24.2]  generate custom elements + source maps finished in 159 ms
[18:24.4]  generate lazy + source maps finished in 287 ms
[18:24.4]  build finished in 1.53 s

Can you provide a step by step description on how to reproduce the problem please?

@christian-bromann christian-bromann added ionitron: needs reproduction This PR or Issue does not have a reproduction case URL and removed triage labels May 20, 2024
Copy link

ionitron-bot bot commented May 20, 2024

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Stencil starter component library and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@gavmck
Copy link
Author

gavmck commented May 21, 2024

@gavmck thanks for raising the issue.

Unfortunately I was not able to verify that exploding build 😉 I created a new Stencil project, build it with v4.14.0 and then again with v4.18.1 and everything was fine:

❯ npm i
npm WARN deprecated puppeteer@21.11.0: < 22.5.0 is no longer supported

added 349 packages, and audited 350 packages in 6s

38 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
❯ npm run build

> issue5780@0.0.1 build
> stencil build

[18:02.0]  @stencil/core
[18:02.1]  v4.14.0 🚡
[18:02.8]  build, issue5780, prod mode, started ...
[18:02.8]  transpile started ...
[18:04.0]  transpile finished in 1.20 s
[18:04.0]  copy started ...
[18:04.0]  generate custom elements + source maps started ...
[18:04.0]  generate lazy + source maps started ...
[18:04.1]  copy finished (0 files) in 107 ms
[18:04.2]  generate custom elements + source maps finished in 213 ms
[18:04.3]  generate lazy + source maps finished in 299 ms
[18:04.3]  build finished in 1.55 s

❯ rm -r node_modules/ package-lock.json
❯ npm i
npm WARN deprecated puppeteer@21.11.0: < 22.5.0 is no longer supported

added 349 packages, and audited 350 packages in 2s

38 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
❯ npm run build

> issue5780@0.0.1 build
> stencil build

[18:22.1]  @stencil/core
[18:22.3]  v4.18.1 🏍
[18:22.8]  build, issue5780, prod mode, started ...
[18:22.9]  transpile started ...
[18:24.0]  transpile finished in 1.19 s
[18:24.0]  copy started ...
[18:24.1]  generate custom elements + source maps started ...
[18:24.1]  generate lazy + source maps started ...
[18:24.1]  copy finished (0 files) in 104 ms
[18:24.2]  generate custom elements + source maps finished in 159 ms
[18:24.4]  generate lazy + source maps finished in 287 ms
[18:24.4]  build finished in 1.53 s

Can you provide a step by step description on how to reproduce the problem please?

@christian-bromann Ah sorry, it's not the build that explodes, it's the install. The previous instructions said to do this.

import { applyPolyfills, defineCustomElements } from 'my-component-library';

applyPolyfills().then(() => {
  defineCustomElements();
});

Now the applyPolyfills function does not exist unless you build for es5 so previous working installs break.

@gavmck
Copy link
Author

gavmck commented May 21, 2024

The type also seems to output that the function exists still

const generateIndexDts = (indexDtsPath: string, componentsDtsPath: string) => {

@christian-bromann christian-bromann self-assigned this May 21, 2024
@christian-bromann
Copy link
Member

christian-bromann commented May 22, 2024

How do you set applyPolyfills ? Can you please provide a reproducible example? It is unknown to me what my-component-library exports here.

@gavmck
Copy link
Author

gavmck commented May 22, 2024

I'll see if I can know one up. applyPolyfills came from the stencil loader

It looks like it was removed from the docs last year and I missed it.

ionic-team/stencil-site@3a910c2#diff-f1f96788acd155196970469f3c5031d518d3800fde982f92412858072a726bcfL518

@christian-bromann
Copy link
Member

That is correct. This has been done due to the fact that Stencil stopped supporting IE, afaik. Do you think we can close this issue then?

@gavmck
Copy link
Author

gavmck commented May 23, 2024

Do we still need to update the type?

@christian-bromann
Copy link
Member

Yes, let me raise a PR for that.

@christian-bromann christian-bromann added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed ionitron: needs reproduction This PR or Issue does not have a reproduction case URL labels May 23, 2024
@christian-bromann
Copy link
Member

Actually we have this planned to remove for Stencil v5.

@Kwooda
Copy link

Kwooda commented May 28, 2024

We have a polyfill problem in our repos with v4.18.1. We have one repo for Stencil that generates the Web components, and another repo that consumes that repo to generate the React components. Any project that attempts to use the React components gets the following error:
export 'applyPolyfills' (imported as 'applyPolyfills') was not found in '@*******/loader' (possible exports: defineCustomElements, setNonce)

@christian-bromann
Copy link
Member

@Kwooda thanks for the feedback, is there a chance you can provide a minimal reproducible example?

@Kwooda
Copy link

Kwooda commented May 28, 2024

I don't know - last time I tried I got slapped by our security team, but I'll see if there's anything I can do.

@christian-bromann
Copy link
Member

Ideally from a new Stencil project, e.g. that you can create via npm init stencil.

@felipefialho
Copy link

If I keep the same structure described in Stencil documentation to generate wrappers of Vue Components and React Components, we have a problem with Stencil 4.18.x to render these components

You can check my real code here

And I have a workaround to work with Vue3 too

In both cases I receive an error because applyPolyfills doesn't exist anymore

image

Can you help us?

@gavmck
Copy link
Author

gavmck commented Jun 6, 2024

If I keep the same structure described in Stencil documentation to generate wrappers of Vue Components and React Components, we have a problem with Stencil 4.18.x to render these components

You can check my real code here

And I have a workaround to work with Vue3 too

In both cases I receive an error because applyPolyfills doesn't exist anymore

image Can you help us?

@felipefialho The answer for us was just to remove all mention of applyPolyfills

gfellerph added a commit to swisspost/design-system that referenced this issue Jun 17, 2024
Apply polyfills apparently got removed as IE is no longer supported: ionic-team/stencil#5780. No need to keep it around anymore.
@christian-bromann christian-bromann added the Stencil v5 This is slated for Stencil v5 (Release date TBD) label Jun 18, 2024
@danielleroux
Copy link

I'm a little bit confused why was this PR (#5725) part of an v4 minor/patch release, for me it looks like a breaking change and should be done in v5.

if (config.buildEs5) {
await copyPolyfills(config, compilerCtx, esmOutputs);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil Stencil v5 This is slated for Stencil v5 (Release date TBD)
Projects
None yet
Development

No branches or pull requests

5 participants