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

feat: add svelte support #170

Merged
merged 7 commits into from
Oct 20, 2020
Merged

Conversation

janoshrubos
Copy link
Collaborator

@janoshrubos janoshrubos commented Oct 15, 2020

Depends on:
NativeScript/NativeScript#8963
NativeScript/nativescript-cli#5418

PR Checklist

What is the current behavior?

N/A

What is the new behavior?

Added svelte support

Fixes/Implements/Closes #[Issue Number].

.DS_Store

*.js.map
*.js
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about including *.js here.
The current svelte template doesn't have it, and neither does the vue one https://github.com/NativeScript/nativescript-app-templates/blob/master/packages/template-blank-vue/.gitignore

Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that this may have bitten us already. The .gitignore file ignores svelte.config.js which is assumed to exist by the webpack plugin https://github.com/NativeScript/NativeScript/pull/8963/files#diff-34201858e4b5b4e6eaad695e47ee0ec6b6ae4b60ccc81bab535438a0520895e8R15

Copy link
Member

@rigor789 rigor789 Oct 16, 2020

Choose a reason for hiding this comment

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

@halfnelson this gitignore is actually for the templates repo - the one that's used/ends up in projects is from packages/template-blank-svelte/tools/dot.gitignore - so I guess both need to change!

.DS_Store

*.js.map
*.js
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be safer to remove the *.js here. I have been bitten before by this excluding other config files

Copy link
Member

Choose a reason for hiding this comment

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

👍 - would ideally refactor all to use files in package.json rather than npmignore

Execute the following command to create an app from this template:

```
ns create my-blank-svelte --template @nativescript/template-blank-svelte
Copy link
Contributor

Choose a reason for hiding this comment

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

if NativeScript/nativescript-cli#5418 is merged, will this become ns create my-blank-svelte --svelte

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, nevermind this is the docs for the template itself

@halfnelson
Copy link
Contributor

Apart from the .gitignore thing and missing svelte.config.js the rest looks great


# NativeScript Template
*.js.map
*.js
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*.js

Copy link
Member

Choose a reason for hiding this comment

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

@halfnelson I believe this should suffice for the config being ignored?

.DS_Store

*.js.map
*.js
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*.js

@rigor789 rigor789 merged commit be5ac28 into NativeScript:master Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants