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

Merge with upstream 4.0.3 #35

Merged
merged 177 commits into from
Jun 29, 2021
Merged

Merge with upstream 4.0.3 #35

merged 177 commits into from
Jun 29, 2021

Conversation

aruniverse
Copy link
Member

@aruniverse aruniverse commented Mar 9, 2021

only conflicts/changes required were with the following files:

  • packages\react-scripts\package.json
  • packages\react-scripts\config\webpack.config.js
  • packages\react-scripts\scripts\build.js
  • packages\react-scripts\scripts\utils\createJestConfig.js
  • packages\react-scripts\README-imodeljs.md

Diff between upstream and our imjs fork: master...imodeljs:sync/4.0.3
Diff between our cra fork 3.x and 4.0 (this pr): 3.x...imodeljs:sync/4.0.3

Note, the only real change betwween 3.x and 4.0.3 is 41a57a9, which bumps the ver of ts-jest

MichaelDeBoey and others added 30 commits March 24, 2020 09:10
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Bumps [acorn](https://github.com/acornjs/acorn) from 6.4.0 to 6.4.1.
- [Release notes](https://github.com/acornjs/acorn/releases)
- [Commits](acornjs/acorn@6.4.0...6.4.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Passing an array with a single entry is not equivalent. This causes Webpack
to generate another wrapper module around the entry. This is just
unnecessary overhead and bytes.
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
@aruniverse
Copy link
Member Author

We need to make a 3.x branch before we merge this into imodeljs

@calebmshafer
Copy link
Member

@aruniverse this looks great. Thanks for picking it up!

Definitely agree that we need a release branch for 3.x as we will/should continue to support it for iModel.js as it doesn't look like there is anything incompatible. Is that your impression as well?

However, the only reason I see people not doing it is the requirement to move up to ESLint 7.x, whereas iModel.js supports both 6.x and 7.x. Any others?

I do want to see why all of the tests fail again :) I really dislike all the red but not needed for this PR.

I'll give it a test run this weekend with the iTwin Viewer CRA template to see how things go.

@calebmshafer
Copy link
Member

@aruniverse, quick update. I merged the other PR for 3.x and then started testing this one out. Other than the iModel.js repo, I tried it with the Desktop Starter. Everything appears to be working well.

I haven't created the release branch yet for 3.x but I was going to name it imodeljs/3.x.

@aruniverse
Copy link
Member Author

Definitely agree that we need a release branch for 3.x as we will/should continue to support it for iModel.js as it doesn't look like there is anything incompatible. Is that your impression as well?

Yea that's my impression as well, imo the major ver bump to cra 4.0 is relatively minor compared to bumping imjs versions, where most of the changes on upstreams end should be compatible with what users have now. Might be an issue if users are pinning down their @types/react version to an earlier version and are using react@16.14.0 because of missing types for the new jsx-transform. (this is what happened in core, but able to get around this by disabling the transform)

However, the only reason I see people not doing it is the requirement to move up to ESLint 7.x, whereas iModel.js supports both 6.x and 7.x. Any others?

If consumers are using jest, they'll need to bump to atleast ts v3.8, but hopefully thats pretty minor too.

Highlight of changes in upstream 4.0: https://github.com/facebook/create-react-app/releases/tag/v4.0.0
https://reactjs.org/blog/2020/10/20/react-v17.html
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-0.html
https://eslint.org/docs/user-guide/migrating-to-7.0.0
https://jestjs.io/blog/2020/05/05/jest-26
https://jestjs.io/blog/2020/01/21/jest-25

@calebmshafer
Copy link
Member

@aruniverse looks like there hasn't been any other releases from CRA since this one. I'm going to go ahead and create the imodeljs/3.x branch now from the latest 3.4.11 release. Then we can go from there.

aruniverse and others added 2 commits June 14, 2021 09:37
* use CopyStaticAssetsPlugin  from bentley/webpack-tools-core that uses copy-webpack-plugin under the hood to deliver all static assets. this also works with the dev server, so strings should finally be localized there!

Co-authored-by: Arun George <aruniverse@users.noreply.github.com>
@calebmshafer
Copy link
Member

@aruniverse lets go ahead and merge this PR this week. I have created the 3.x branch for all future releasing so this should be clear to merge.

We still need to figure out the issue with the latest change to the copy webpack plugin. That is still holding up the official usage of it in the imodeljs repo. Have yo made any progress on it? If not, maybe @wgoehrig and I can help this week.

@aruniverse
Copy link
Member Author

We still need to figure out the issue with the latest change to the copy webpack plugin. That is still holding up the official usage of it in the imodeljs repo.

Yea i was just thinking about this, and pinged you on the side haha
I haven't made any progress on it since i returned for vacation. Will return to investigating it tomorrow, and most likely creating an issue in that pkg's repo

@calebmshafer
Copy link
Member

@aruniverse sounds good, I'm thinking we just back out the update for now or make it optional behind a flag.

…k to old plugin. 3.4.13 (#45)

* Add a flag called DISABLE_NEW_ASSET_COPY to turn off new asset copying plugin

* Bump version 3.4.13
Copy link
Member

@calebmshafer calebmshafer left a comment

Choose a reason for hiding this comment

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

I think the process we laid out before is to make a merge commit here rather than squash so let me know when you're ready.

I'd say just remove the -dev and we're set

@calebmshafer calebmshafer merged commit d976aca into imodeljs Jun 29, 2021
@calebmshafer calebmshafer deleted the sync/4.0.3 branch June 29, 2021 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.