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: update nodeModulesDir config JSON schema #25653

Merged
merged 4 commits into from
Sep 18, 2024
Merged

Conversation

marvinhagemeister
Copy link
Contributor

@marvinhagemeister marvinhagemeister commented Sep 16, 2024

Ref #25380

{
"description": "Enables or disables the use of a local node_modules folder for npm packages. Alternatively, use the `--node-modules-dir` flag or override the config via `--node-modules-dir=false`. Requires Deno 1.34 or later.",
"type": "boolean",
"deprecated": true
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe we should flip this to true after the deno 2.0 release? This will go live immediately after merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good point

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

This setting hasn't landed in v1.46 IIRC so why are we changing it in config-file.v1.json? Shouldn't we add a new version of the schema file?

@dsherret
Copy link
Member

This setting hasn't landed in v1.46 IIRC so why are we changing it in config-file.v1.json? Shouldn't we add a new version of the schema file?

I think that's going to be too much work to do before Deno 2.0 because this is managed by the vscode extension independent of the CLI. I think we should look into that post Deno 2.0.

@bartlomieju
Copy link
Member

But now people who use v1.4X will have squiggly lines for nodeModulesDir and deprecated "files" config. It shouldn't be too much work to select the proper schema file based on the version. @nayeemrmn can you chime in here?

@dsherret
Copy link
Member

But now people who use v1.4X will have squiggly lines for nodeModulesDir and deprecated "files" config.

@bartlomieju No. See #25653 (comment) -- it was flipped

@dsherret
Copy link
Member

Oh, files config was already removed, but we've been warning about that one for ages so it's probably fine.

@petamoriken
Copy link
Contributor

Will TypeScript 5.6 be supported in Deno v1.x? If not, it may be better to separate schema files because of new compilerOptions such as "strictBuiltinIteratorReturn".

@bartlomieju bartlomieju merged commit 28dd928 into main Sep 18, 2024
17 checks passed
@bartlomieju bartlomieju deleted the config-schema-2 branch September 18, 2024 20: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.

4 participants