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] rename dotfiles to avoid template problems #2990

Merged
merged 2 commits into from
Dec 12, 2021

Conversation

Conduitry
Copy link
Member

Fixes #2952.

Rather than adding another special case for .npmrc like there already is for .gitignore, this simply renames all dotfiles to instead begin with DOT- when the create-svelte package is being built, and un-renames them when bootstrapping a new project with the CLI. This doesn't currently handle dotfiles other than at the top level of a particular template, but we don't have any of those. I didn't include that for simplicity, but if other folks think that would be a good idea to handle now while we're thinking of it, I'd be amenable to that.

Please don't delete this checklist! 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
  • 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 pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@Conduitry Conduitry added bug Something isn't working pkg:create-svelte labels Dec 6, 2021
@changeset-bot
Copy link

changeset-bot bot commented Dec 6, 2021

🦋 Changeset detected

Latest commit: 774ebc7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
create-svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dummdidumm
Copy link
Member

I don't see the renamed dot-files in this PR

@Conduitry
Copy link
Member Author

They're not renamed in this repo, and they shouldn't need to be. They're renamed as they're copied into the dist folder that gets packaged up, and then renamed back by the CLI when they're copied to the user's directory. Try this out locally and examine the contents of npm pack and the results of running the CLI.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Ah now I understand, thanks!

Copy link
Contributor

@dreitzner dreitzner left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -134,7 +134,7 @@ async function main() {
*/
function write_template_files(template, typescript, name, cwd) {
const dir = dist(`templates/${template}`);
copy(`${dir}/assets`, cwd, (name) => name.replace('gitignore', '.gitignore'));
copy(`${dir}/assets`, cwd, (name) => name.replace('DOT-', '.'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use regex here too, for the off chance that there will ever be any file containing DOT- in it's name...

Suggested change
copy(`${dir}/assets`, cwd, (name) => name.replace('DOT-', '.'));
copy(`${dir}/assets`, cwd, (name) => name.replace(/^DOT-/, '.'));

Copy link
Contributor

@dreitzner dreitzner Dec 7, 2021

Choose a reason for hiding this comment

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

DOT- could also be extracted as constant into packages/create-svelte/utils.js

@benmccann
Copy link
Member

I didn't review, but with four approvals, this seems safe to merge 😄

@benmccann benmccann merged commit 76d26ec into sveltejs:master Dec 12, 2021
@Conduitry Conduitry deleted the gh-2952 branch December 12, 2021 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:create-svelte
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] npm init svelte@next - some files missing
6 participants