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

dev script creates unnecessary env.d.ts file #6013

Closed
1 task done
loucyx opened this issue Jan 28, 2023 · 21 comments
Closed
1 task done

dev script creates unnecessary env.d.ts file #6013

loucyx opened this issue Jan 28, 2023 · 21 comments
Assignees
Labels
- P2: has workaround Bug, but has workaround (priority)

Comments

@loucyx
Copy link
Contributor

loucyx commented Jan 28, 2023

What version of astro are you using?

2.0.2

Are you using an SSR adapter? If so, which one?

vercel

What package manager are you using?

pnpm

What operating system are you using?

MacOS

Describe the Bug

When running pnpm dev to run the server locally on an existing Astro project of mine I just upgraded, the script creates a new env.d.ts file in my src folder, with this on it:

/// <reference types="astro/client" />

This is not necessary at all because my tsconfig.json file already has a compilerOptions.types (more about this option here) set to use astro/client:

{
	// ...
	"compilerOptions": {
		"types": ["astro/client"],
		// ...
	},
	// ...
}

The dev script shouldn't create that file in every run, and even less so if already included the types in tsconfig.json. If we agree creating new files in each run of dev is not good, I'm willing to take a look at said script in astro's code and fix it for you, or even update the tsconfig.json or jsconfig.json file you generate to include the types there.

Link to Minimal Reproducible Example

https://gist.github.com/lukeshiru/a171e9fdd0c9b78296fbd41f2fef1373

Participation

  • I am willing to submit a pull request for this issue.
@bholmesdev bholmesdev self-assigned this Jan 30, 2023
@bholmesdev bholmesdev added the - P2: has workaround Bug, but has workaround (priority) label Jan 30, 2023
@bholmesdev
Copy link
Contributor

Hey @lukeshiru! Yes, we now auto-generate an env.d.ts if none is present to ensure astro/client is wired up. We currently don't check whether you've wired this up manually view a tsconfig.json. I'm speaking with our maintainers to see what the expected behavior should be.

@loucyx
Copy link
Contributor Author

loucyx commented Jan 30, 2023

Hi @bholmesdev! Thanks for taking this to the maintainers! For now, I added that file to my .gitignore. Is kinda annoying to get a file created in my src directory every time I run dev (more so when I have that solved from package.json), but is not the end of the world.

If y'all need an extra set of hands to figure this out, let me know. Happy to help! ☺️

@bholmesdev
Copy link
Contributor

@lukeshiru Understood! Speaking with core, we agree that src/env.d.ts is an expected file for Astro 2.0 that you'd need to manually ignore. We've also received reports of language tools and types breaking in the past by setting astro/client from types, so we're encouraging src/env.d.ts as a best practice.

Still, I'm curious why your project must exclude this file. Is it preference, or are they're technical reasons an env.d.ts is a no-go?

@loucyx
Copy link
Contributor Author

loucyx commented Feb 6, 2023

@bholmesdev is almost a "preference" thing, but not quite:

  1. If you add "types": ["astro/client"] to the compilerOptions of tsconfig.json, you don't need an additional file in src, which is cleaner for everyone, and you're already creating a tsconfig.json file anyways.
  2. Creating a new file in src every time the developer runs dev is far from ideal. Therefore, file creation on dev controlled folders such as src should only happen in the initial setup.

To clarify, I always use TypeScript (used it since it was released back in 2012), so it is not that I'm a vanilla JS dev complaining about TS, but more of a TS dev with a concern about incorrect configs and a "not bad, but definitely weird DX".

@bholmesdev
Copy link
Contributor

bholmesdev commented Feb 7, 2023

@lukeshiru Understood. To respond to each of those points:

  1. I'll confess src/env.d.ts over "types" is a fairly nuanced issue, though we've found declaring astro/client types from an env.d.ts reference is more reliable. I'll defer to @Princesseuh for further details on that choice. Though introducing a new file feels annoying at first, we think making env.d.ts a home for these types is safest in the long run.
  2. This file is only generated the first time! Since it's intended to not be gitignored, we decided this was expected behavior. The alternative would be to only generate this file on build, which would mean a less smooth onboarding experience for those trying 2.0 or content collections.

And I recognize this isn't well-documented behavior. I think our TypeScript docs should clearly call out how src/env.d.ts is a generated file that should be present in every Astro 2.0 project.

Once this is documented, I plan to close as a "will not fix." We can reopen if a more compelling reason to skip env generation comes up!

@Princesseuh
Copy link
Member

Princesseuh commented Feb 7, 2023

The reason we use src/env.d.ts over types is the same reason other frameworks (as far as I know) also use a file like this (namely Vite, Next too I believe previously and I'm sure others): using types makes types acquisition explicit.

To quote TypeScript's documentation:

By default all visible ”@types” packages are included in your compilation. Packages in node_modules/@types of any enclosing folder are considered visible. For example, that means packages within ./node_modules/@types/, ../node_modules/@types/, ../../node_modules/@types/, and so on.

If types is specified, only packages listed will be included in the global scope.

That means that by default, just installing, ex: @types/node like you would do in other TypeScript projects wouldn't work in Astro, it would need to be explicitly added to types. We decided that creating an additional file was better than silently breaking things for users who don't know this detail of types. For context, we kinda used to use types and we'd get issues about this specific thing, so there's definitely a precedent here.

@loucyx
Copy link
Contributor Author

loucyx commented Feb 7, 2023

Oh ok, I get it. The decision of using an extra file instead of using the configs that are already there is mainly to avoid breaking the implicit import of global types (such as @types/node) for folks that don't know how the types config works in compilerOptions.

I thought that maybe creating that file if is not there every time dev is run was undesirable compared to creating it in the first setup (not on build either, just when you create a new Astro project).

But no worries then, I have the workaround for my setup, so in my Astro projects I'll just keep that file in my .gitignore and use configs because that's what I prefer.

Thanks for explaining your reasoning behind the decision, folks! I love what you're doing with Astro, thanks a lot for all your work! ✨

@loucyx loucyx closed this as completed Feb 7, 2023
alex-grover added a commit to alex-grover/alexgrover.me-old that referenced this issue Feb 25, 2023
github-actions bot pushed a commit to alex-grover/alexgrover.me-old that referenced this issue Feb 25, 2023
@spacedawwwg
Copy link

spacedawwwg commented Mar 13, 2023

Can this not be configurable? I like to maintain all my *.d.ts files in a @types directory - and it's really annoying this one file sits outside of that (when i could just manually add it)

@fuckadey
Copy link

Another weird thing – this file shouldn't be there when you don't use typescript, but for some reason it is)

@Princesseuh
Copy link
Member

Another weird thing – this file shouldn't be there when you don't use typescript, but for some reason it is)

Astro is TypeScript only. Without this file you'd get editor errors when importing certain things / using certain APIs.

@fuckadey
Copy link

fuckadey commented Aug 27, 2023

Another weird thing – this file shouldn't be there when you don't use typescript, but for some reason it is)

Astro is TypeScript only. Without this file you'd get editor errors when importing certain things / using certain APIs.

Original concern is the tool messing up with user's repository because of some internal machinery. A minor distraction, yes, to go and exclude it from VCS, but again, still a distraction.

@a-y-u-s-h
Copy link

Any update on this? I can confirm that the behaviour of generating env.d.ts while using Javascript still exists. Would be nice to have that disappear from my project. Distracts the eye, as someone mentioned earlier in the thread.

@Princesseuh
Copy link
Member

Any update on this? I can confirm that the behaviour of generating env.d.ts while using Javascript still exists. Would be nice to have that disappear from my project. Distracts the eye, as someone mentioned earlier in the thread.

You can configure your editor to hide the file if you want. Astro is TypeScript only and this file (or an equivalent) is necessary to power several of Astro's features. It's unlikely we'll get rid of it, or if we do, we'd just move it somewhere else.

@a-y-u-s-h
Copy link

Any update on this? I can confirm that the behaviour of generating env.d.ts while using Javascript still exists. Would be nice to have that disappear from my project. Distracts the eye, as someone mentioned earlier in the thread.

You can configure your editor to hide the file if you want. Astro is TypeScript only and this file (or an equivalent) is necessary to power several of Astro's features. It's unlikely we'll get rid of it, or if we do, we'd just move it somewhere else.

Sounds nice, it's a practice that I do in Remix to move cache into node_modules/remix. can it be moved to node_modules/astro by the framework or does vite provide a functionality to do it automatically through config?

@Princesseuh
Copy link
Member

We can't really move it to node_modules because editors have trouble recognizing file updates from there. We can't move it to a hidden folder without another workaround (which would need a tsconfig.json setting) either.

There's no settings to change this at this time, maybe in the future we'll do something, but this is very low priority since most people don't really mind

@a-y-u-s-h
Copy link

I see, I see. Can the issue still be remained open though? It still feels like a teeny-tiny minor improvement over default behaviour not worthy of getting forgotten in a closed issue.

@Princesseuh
Copy link
Member

We reserve our issue tracker for technical issues, for suggestions feel free to create a discussion on our roadmap repo: https://github.com/withastro/roadmap so that the community can vote on it and we can prioritize it if it becomes popular

@thewaliii
Copy link

I don't like this file. How to get rid of it?

shubham-padia added a commit to shubham-padia/zulip that referenced this issue Jun 28, 2024
We are adding MDX files to `.gitignore` for now since they are
just a result of a build process, once the migration is done,
we will not ignore them.
We're using PNPM workspaces to manage this project.
The new project's tsconfig.json has been copied from the current
root tsconfig.json while omitting some details that are only
relevant to that project.
`help-beta/src/env.d.ts` is a type declaration file auto-generated
by Astro. See withastro/astro#6013.
@codethief
Copy link

codethief commented Jul 26, 2024

The issue with placing the type import in a env.d.ts file is that AFAICT it gets pulled in automatically as long as the tsconfig contains something like "include": ["**/*"] – which it does by default.

However, my understanding is that astro/client types should really only be available in code executed by Astro, i.e. not in tests, Storybook stories, and maybe not even inside Vue components?

In my case, I want to use Astro in a project that employs project references to separate type checking of client-side code, server-side code, node helper scripts, test code, stories etc. While astro check does not support project references yet, the rest of my code does (it gets type-checked by vue-tsc), so I'd rather not have astro/client types fly around where they shouldn't be available and rather only include them in the tsconfig I set up for my Astro code.

Long story short: I'd vote for not creating an env.d.ts file automatically every single time during dev and for re-opening this ticket.

For now I'll probably work around this by explicitly excludeing the file in my non-Astro tsconfigs.

shubham-padia added a commit to shubham-padia/zulip that referenced this issue Jul 29, 2024
We are adding MDX files to `.gitignore` for now since they are
just a result of a build process, once the migration is done,
we will not ignore them.
We're using PNPM workspaces to manage this project.
The new project's tsconfig.json has been copied from the current
root tsconfig.json while omitting some details that are only
relevant to that project.
`help-beta/src/env.d.ts` is a type declaration file auto-generated
by Astro. See withastro/astro#6013.
timabbott pushed a commit to shubham-padia/zulip that referenced this issue Aug 3, 2024
We are adding MDX files to `.gitignore` for now since they are
just a result of a build process, once the migration is done,
we will not ignore them.
We're using PNPM workspaces to manage this project.
The new project's tsconfig.json has been copied from the current
root tsconfig.json while omitting some details that are only
relevant to that project.
`help-beta/src/env.d.ts` is a type declaration file auto-generated
by Astro. See withastro/astro#6013.
timabbott pushed a commit to zulip/zulip that referenced this issue Aug 3, 2024
We are adding MDX files to `.gitignore` for now since they are
just a result of a build process, once the migration is done,
we will not ignore them.
We're using PNPM workspaces to manage this project.
The new project's tsconfig.json has been copied from the current
root tsconfig.json while omitting some details that are only
relevant to that project.
`help-beta/src/env.d.ts` is a type declaration file auto-generated
by Astro. See withastro/astro#6013.
afeefuddin pushed a commit to afeefuddin/zulip that referenced this issue Aug 10, 2024
We are adding MDX files to `.gitignore` for now since they are
just a result of a build process, once the migration is done,
we will not ignore them.
We're using PNPM workspaces to manage this project.
The new project's tsconfig.json has been copied from the current
root tsconfig.json while omitting some details that are only
relevant to that project.
`help-beta/src/env.d.ts` is a type declaration file auto-generated
by Astro. See withastro/astro#6013.
@ariley
Copy link

ariley commented Aug 23, 2024

I installed then removed typescript. But the env.d.ts file keeps getting generated upon build. How can I disable that? I don't think I need it.

@vrabe
Copy link

vrabe commented Aug 29, 2024

Check #11859, env.d.ts will be replaced by tsconfig configs since v5.0. There will be no more env.d.ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority)
Projects
None yet
Development

No branches or pull requests

10 participants