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: backwards compatibility for types #8721

Merged
merged 19 commits into from
Jun 18, 2023
Merged

fix: backwards compatibility for types #8721

merged 19 commits into from
Jun 18, 2023

Conversation

gtm-nayan
Copy link
Contributor

HEADS UP: BIG RESTRUCTURING UNDERWAY

The Svelte repo is currently in the process of heavy restructuring for Svelte 4. After that, work on Svelte 5 will likely change a lot on the compiler aswell. For that reason, please don't open PRs that are large in scope, touch more than a couple of files etc. In other words, bug fixes are fine, but feature PRs will likely not be merged.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@vercel
Copy link

vercel bot commented Jun 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
svelte-dev-2 ❌ Failed (Inspect) Jun 15, 2023 6:52pm

@gtm-nayan
Copy link
Contributor Author

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 11, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ❌ failure
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ❌ failure
rollup-plugin-svelte ❌ failure
skeleton ❌ failure
svelte-eslint-parser ❌ failure
svelte-loader ❌ failure
svelte-preprocess ❌ failure
sveltekit ❌ failure
vite-plugin-svelte ❌ failure

@gtm-nayan
Copy link
Contributor Author

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 11, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ❌ failure

@gtm-nayan
Copy link
Contributor Author

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 11, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ❌ failure

@gtm-nayan
Copy link
Contributor Author

image

@gtm-nayan
Copy link
Contributor Author

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 11, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ⏹️ cancelled
mdsvex ⏹️ cancelled
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ⏹️ cancelled
vite-plugin-svelte ✅ success

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 11, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ✅ success

fs.mkdirSync('types/faux', { recursive: true });
fs.readdirSync('src/runtime', { withFileTypes: true })
.filter((dirent) => dirent.isDirectory())
.forEach((dirent) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this include internal? Should be filtered out probably

@gtm-nayan
Copy link
Contributor Author

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 12, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ✅ success

@gtm-nayan
Copy link
Contributor Author

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 12, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ✅ success

@benmccann benmccann added this to the 4.x milestone Jun 12, 2023
@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 16, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ❌ failure
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ❌ failure
rollup-plugin-svelte ❌ failure
skeleton ❌ failure
svelte-eslint-parser ❌ failure
svelte-loader ❌ failure
svelte-preprocess ❌ failure
sveltekit ❌ failure
vite-plugin-svelte ❌ failure

@dummdidumm
Copy link
Member

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 16, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ❌ failure
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ✅ success

@dummdidumm
Copy link
Member

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 16, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ✅ success
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ✅ success

@dummdidumm
Copy link
Member

I think most things are fixed, one is missing: SvelteComponent is exported publicly, not SvelteComponentDev. dts-buddy does not seem to handle the export { Sth as SthElse } from '..' case, it just ignores the aliasing. I looked into what it means just exposing SvelteComponent but that would mean changes to the runtime code which are invalid strictly speaking and which increases the bundle size by a tiny amount for no good reason. That's because we need to type a constructor and type some instance fields, both are not actually needed by the Svelte runtime. @Rich-Harris / @gtm-nayan if one of you could look into dts-buddy that would be great, else I'll try to tackle this on monday.

@dummdidumm
Copy link
Member

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 16, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ✅ success
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ✅ success

@dummdidumm
Copy link
Member

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 17, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ✅ success
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ❌ failure
vite-plugin-svelte ✅ success

@dummdidumm
Copy link
Member

dummdidumm commented Jun 17, 2023

Ok I found a workaround for the dts-buddy bug. Ecosystem CI run looks mostly good now. SvelteKit tests - no idea why they failed, maybe unrelated. language-tools: one needs a change in language-tools due to where the declare module '*.svelte' declaration lives now, the other failure needs investigation.

@dummdidumm
Copy link
Member

/ecosystem-ci run

@svelte-ecoystem-ci
Copy link

svelte-ecoystem-ci bot commented Jun 18, 2023

📝 Ran ecosystem CI: Open

suite result
eslint-plugin-svelte ✅ success
language-tools ❌ failure
mdsvex ✅ success
prettier-plugin-svelte ✅ success
rollup-plugin-svelte ✅ success
skeleton ✅ success
svelte-eslint-parser ✅ success
svelte-loader ✅ success
svelte-preprocess ✅ success
sveltekit ✅ success
vite-plugin-svelte ✅ success

@dummdidumm dummdidumm marked this pull request as ready for review June 18, 2023 19:10
@dummdidumm
Copy link
Member

Nice, ecosystem-ci is green again (that red cross from language tools is a timeout, that happens from time to time)

@dummdidumm dummdidumm merged commit cc82d5d into version-4 Jun 18, 2023
@dummdidumm dummdidumm deleted the types-backcompat branch June 18, 2023 19:12
This was referenced Jun 18, 2023
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.

4 participants