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

chore(build): remove obsolete rollup code #5616

Closed
wants to merge 1 commit into from

Conversation

christian-bromann
Copy link
Member

What is the current behavior?

We have some duplicated code for creating build files in rollup. This is the first PR of many to remove these obsolete artifacts.

What is the new behavior?

Don't bundle these dev server assets anymore as have migrated this to Rollup.

Documentation

Does this introduce a breaking change?

  • Yes
  • No

Testing

n/a

Other information

n/a

Copy link
Contributor

github-actions bot commented Apr 2, 2024

--strictNullChecks error report

Typechecking with --strictNullChecks resulted in 1138 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/testing/puppeteer/puppeteer-element.ts 21
src/runtime/client-hydrate.ts 20
src/screenshot/connector-base.ts 19
src/runtime/vdom/vdom-render.ts 17
src/dev-server/request-handler.ts 15
src/compiler/prerender/prerender-optimize.ts 14
src/compiler/sys/stencil-sys.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/connected-callback.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
src/compiler/transpile/transpile-module.ts 12
src/mock-doc/test/attribute.spec.ts 12
Our most common errors
Typescript Error Code Count
TS2322 361
TS2345 345
TS18048 204
TS18047 82
TS2722 37
TS2532 24
TS2531 21
TS2454 14
TS2790 11
TS2352 9
TS2769 8
TS2538 8
TS2416 7
TS2493 3
TS18046 2
TS2684 1
TS2430 1

Unused exports report

There are 14 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 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 61 satisfies
src/compiler/types/validate-primary-package-output-target.ts 61 Record
src/testing/puppeteer/puppeteer-declarations.ts 485 WaitForEventOptions
src/compiler/sys/fetch/write-fetch-success.ts 7 writeFetchSuccessSync

Copy link
Contributor

github-actions bot commented Apr 2, 2024

PR built and packed!

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

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

unzip -d ~/Downloads ~/Downloads/stencil-core-4.14.0-dev.1712066930.e7d7301.tgz.zip && npm install ~/Downloads/stencil-core-4.14.0-dev.1712066930.e7d7301.tgz

@rwaskiewicz
Copy link
Member

I think for us to take this on, we should wait until the rollup -> esbuild migration is complete. That way in case we have to roll something back, it's one less commit to revert - unless there's something we're changing here we should do before/during the migration?

@christian-bromann
Copy link
Member Author

unless there's something we're changing here we should do before/during the migration?

Not really, it is just duplicated code which will get removed anyway if we removed the bundles directory.

@rwaskiewicz
Copy link
Member

@christian-bromann Sounds good, let's hold off on this then until we have a few esbuild-based production builds out the door

@christian-bromann
Copy link
Member Author

Closing in favor of #5698

@christian-bromann christian-bromann deleted the cb/remove-rollup-code branch May 7, 2024 21:38
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

Successfully merging this pull request may close these issues.

None yet

2 participants