Initializer for react-native sample
#879
Conversation
This reverts commit 451c35d.
packages/create-sitecore-jss/src/initializers/react-native/prompts.ts
Outdated
Show resolved
Hide resolved
packages/create-sitecore-jss/src/templates/react-native/package.json
Outdated
Show resolved
Hide resolved
|
|
||
| android_build_config( | ||
| name = "build_config", | ||
| package = "com.<%- appName %>", |
There was a problem hiding this comment.
Looks like there were a lot of additional appName replacements (vs what we were replacing previously - basically just package.json and sitecore.config). Only concern here is that we don't do regression testing for React Native (automated or manual) as part of release activities. Any risk?
There was a problem hiding this comment.
Yeah I could see this being a bit spooky. Another concern/question I had about this is that these package names come from the folder names I believe, and they were originally PascalCase but at the moment they would be skewer or however the user inputs the name. Do we need those folders to be PascalCase (is this a Java standard practice? I think so...)
@illiakovalenko do you know if the case for the directories matter? Just noticed another foldername in the templates with the default appname, android/app/src/java/com/basicreactnativesample, should this be replaced too and will hyphens break it?
There was a problem hiding this comment.
I can't answer for sure without testing
There was a problem hiding this comment.
Based on what I'm hearing, sounds like we should err on the side of keeping things as they were named before.
packages/create-sitecore-jss/src/templates/react-native/package.json
Outdated
Show resolved
Hide resolved
packages/create-sitecore-jss/src/initializers/react-native/prompts.ts
Outdated
Show resolved
Hide resolved
packages/create-sitecore-jss/src/initializers/react-native/args.ts
Outdated
Show resolved
Hide resolved
| ...answers, | ||
| }; | ||
|
|
||
| const FILTER_REGEXP = /.(jar)$/; |
There was a problem hiding this comment.
As I understand, we should skip files with .jar extension. What is the reason?
There was a problem hiding this comment.
Same issue as .pdfs where it has %> encoded so we want to copy it over, but skip EJS.
It actually needs to go this logic here instead of being skipped completely like it is now:
Should we add jar to the ASSET_REGEX?
There was a problem hiding this comment.
do we want this^ to be:
const COPY_ONLY_REGEX = /\.(gif|jpg|jpeg|tiff|png|svg|ashx|ico|pdf|jar)$/;or something similar?
There was a problem hiding this comment.
Sounds like a sensible name/change to me. We have something to make this better (handle "skip file for copy" and "skip file for ejs processing" separately) in our improvements task.
packages/create-sitecore-jss/src/initializers/react-native/index.ts
Outdated
Show resolved
Hide resolved
|
|
||
| android_build_config( | ||
| name = "build_config", | ||
| package = "com.<%- appName %>", |
There was a problem hiding this comment.
I can't answer for sure without testing
packages/create-sitecore-jss/src/templates/react-native/ios/{{appName}}-tvOS/Info.plist
Show resolved
Hide resolved
DO NO MERGE!!Description / Motivation
Testing Details
Types of changes