Skip to content

V3 mods #1

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

Merged
merged 3 commits into from
Aug 17, 2017
Merged

V3 mods #1

merged 3 commits into from
Aug 17, 2017

Conversation

JamieDixon
Copy link

Modifications to support V3

const reactScriptsLinked =
fs.existsSync(reactScriptsPath) &&
fs.lstatSync(reactScriptsPath).isSymbolicLink();
// const ownPackageJson = require('../package.json');
Copy link
Author

Choose a reason for hiding this comment

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

I think this can be uncommented now. Was useful when react-scripts was in our codebase but now we're CRA, these checks are legit.


module.exports = {
locales,
languages: locales.map(l => l.replace(/(-.*)$/, '')) // en-gb -> en

Choose a reason for hiding this comment

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

What does languages represent? Used for? Is it ok to have duplicate values in the list (e.g. both en-gb and en-us will create en)

Copy link
Author

Choose a reason for hiding this comment

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

As far as I know it's only used to build up a regex that determines which react-intl language locale packs get loaded. The duplication won't matter in that case.

@@ -59,6 +63,7 @@
"whatwg-fetch": "2.0.3"
},
"devDependencies": {
"path": "^0.12.7",
Copy link

Choose a reason for hiding this comment

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

this should go too

"name": "react-scripts",
"version": "1.0.11",
"name": "@connected-home/react-scripts",
"version": "1.0.11.1",
Copy link

Choose a reason for hiding this comment

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

this looks unintended? did you mean to write 1.0.11-1 ?

you can use x.y.z followed by - + somestring to create prerelease versions)
e.g 1.0.11-1, 1.0.11-2

see http://semver.org/#spec-item-9 for proper words on it

Copy link
Author

Choose a reason for hiding this comment

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

I was just trying to create a new version number so I could publish to npm. Ideally I would just leave the version number alone but I couldn't figure out how to overwrite an npm publish.

Copy link

Choose a reason for hiding this comment

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

ah ok, you can't overwrite a published version. You can only delete them. prerelease versioning is your friend here.

@sandfox
Copy link

sandfox commented Aug 16, 2017

The only other changes that I would like making are changing the output filenames so they are full-jazz-hands wired SHA256 hashes instead of the tired 8-char hashes, I'm happy do do that and drop it on here as another commit.

@sandfox
Copy link

sandfox commented Aug 16, 2017

We should also try to rebase these commits down a little considering we're going to have to keep bringing work from upstream (facebooks repo), it should make future merging / understanding-wtf-is-going-on easier.

@JamieDixon
Copy link
Author

JamieDixon commented Aug 16, 2017

@sandfox RE SHA256 hashes in filenames, is this an issue for the favicon package over at https://github.com/ConnectedHomes/hivehome-webapp-favicons-webpack-plugin ?

Might be related to https://github.com/ConnectedHomes/hivehome-webapp-favicons-webpack-plugin/issues/5

@JamieDixon
Copy link
Author

@sandfox Rebasing the commits down sounds good. Some of these are "oops" kinda commits.

Modify prod build

Move package over to @connected-home

Downgrade jest and babel-jest to support V3

Undo commented template section. Should work now that were referencing react-scripts as part of CRA rather than a folder in our project

Delete yarn.lock file and use npm for this proj instead

Remove fs package that was added by us for some reason

Add HivehomeWebappFaviconsWebpackPlugin with proper config

Remove path package from devDependencies
Adjust a few bits so that Jest works properly with V3

Add property to jest config for matching jest files in this lower version of jest
…the end

Bump favicon plugin ref to 2.0.0

Change favicon config to use ios rather than iphone key

Bump versions
@JamieDixon JamieDixon merged commit e73606a into master Aug 17, 2017
@JamieDixon JamieDixon mentioned this pull request Aug 17, 2017
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