Skip to content

Fix noFallthroughCasesInSwitch/jsx object is not extensible - #9921

Merged
ianschmitz merged 2 commits into
react:masterfrom
ryota-murakami:fix-4.0.0-typescript-broken
Oct 30, 2020
Merged

Fix noFallthroughCasesInSwitch/jsx object is not extensible#9921
ianschmitz merged 2 commits into
react:masterfrom
ryota-murakami:fix-4.0.0-typescript-broken

Conversation

@ryota-murakami

Copy link
Copy Markdown
Contributor

Relative Issues #9868#9429

I fixed broken yarn start that cause react-scripts can't handle correctly noFallthroughCasesInSwitch and jsx compileOptions.

Oct-27-2020 16-12-58

Thank you for your decent review 😀

@mrmckebmrmckeb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iansu@ianschmitz I've had this reported to me privately too, this seems like a change to the behaviour of ts.readConfigFile where the object is no longer extensible.

@mrmckebmrmckeb closed this Oct 27, 2020
@mrmckeb

Copy link
Copy Markdown
Contributor

Re-running CI.

@mrmckebmrmckeb reopened this Oct 27, 2020
@benneq

Copy link
Copy Markdown
Contributor

@ryota-murakami Have you check the inner properties of compilerOptions? Those should be frozen, too. Though a simple appTsConfig.compilerOptions = { ...appTsConfig.compilerOptions }; might not work.

@ianschmitzianschmitz added this to the 4.0.1 milestone Oct 28, 2020
Comment threadpackages/react-scripts/scripts/utils/verifyTypeScriptSetup.js Outdated
@ryota-murakami

Copy link
Copy Markdown
ContributorAuthor

@benneq@KonstantinSimeonov I'm sorry for slow response.

Have you check the inner properties of compilerOptions? Those should be frozen, too. Though a simple appTsConfig.compilerOptions = { ...appTsConfig.compilerOptions }; might not work.

I've checked inner properties of compilerOptions are frozen.
Bellow screenshot showing Object.isFrozen(appTsConfig.compilerOptions.module) return true after destructured property of appTsConfig.compilerOptions on new Object.

appConfig

And I'll share video clip that recorded running bellow code without Error.

if(appTsConfig.compilerOptions==null){appTsConfig.compilerOptions={};firstTimeSetup=true;}else{appTsConfig.compilerOptions={ ...appTsConfig.compilerOptions};for(constoptioninappTsConfig.compilerOptions){if(!Object.isFrozen(option))newError('appTsConfig.compilerOptions option should be frozen')}}

https://www.dropbox.com/s/912pp3xudr9rn9s/Screen%20Recording%202020-10-30%20at%2021.40.54.mov?dl=0

@KonstantinSimeonov

KonstantinSimeonov commented Oct 30, 2020

Copy link
Copy Markdown
Contributor

@ryota-murakami

for(constoptioninappTsConfig.compilerOptions){if(!Object.isFrozen(option))newError('appTsConfig.compilerOptions option should be frozen')}

This is iterating over the keys of the object, which are strings and are always frozen and is not checking whether the properties are frozen or not 😛 . I think @benneq 's idea was to make sure that no crashes would occur if any of appTsConfig.compilerOptions's properties is frozen. Although I think that this is unlikely, you can safeguard against it by deep cloning the whole object.

Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
@ryota-murakami

Copy link
Copy Markdown
ContributorAuthor

@KonstantinSimeonov Thanks, that iterator is garbage 😅
And merge your suggested solution. I confirm that working.

But I don't understand why guys care about property frozen status?
This problem root is Object isn't extensible, co script couldn't add suggested config.
I think it doesn't matter frozen status 🤔

@ianschmitzianschmitz changed the title 4.0.0 Fix noFallthroughCasesInSwitch/jsx object is not extensible err…Fix noFallthroughCasesInSwitch/jsx object is not extensibleOct 30, 2020
@ianschmitz
ianschmitz merged commit 3a98ed1 into react:masterOct 30, 2020
@ianschmitz

Copy link
Copy Markdown
Contributor

Thanks folks!

@ryota-murakami

ryota-murakami commented Nov 1, 2020

Copy link
Copy Markdown
ContributorAuthor

@benneq@KonstantinSimeonov I got it why should we handle isFroze() result of each compileOptions.
If isFroze(someOption) === true and original config from reading files value is different from CRA suggested,
the script couldn't apply change to suggested value.

Thank you for your support about this PR!

@vegawong

Copy link
Copy Markdown

@ianschmitz should it make a new release version?

oklas added a commit to oklas/react-app-alias that referenced this pull request Nov 21, 2020
@cseas

Copy link
Copy Markdown

Can we please have a release with this fix? The problem is occurring for a lot of people in #9868

blackarctic added a commit to blackarctic/create-react-app that referenced this pull request Nov 25, 2020
* Fix noFallthroughCasesInSwitch/jsx object is not extensible (react#9921)
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
* Add logo license to README
* Remove trailing space in reportWebVitals.ts (react#10040)
* docs: add React Testing Library as a library requiring jsdom (react#10052)
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
* Increase Workbox's maximumFileSizeToCacheInBytes (react#10048)
* Create FUNDING.yml
* replace inquirer with prompts (react#10083)
- remove `react-dev-utils/inquirer` public import
* Prepare 4.0.1 release
* Prepare 4.0.1 release
* Publish
- cra-template-typescript@1.1.1
- cra-template@1.1.1
- create-react-app@4.0.1
- react-dev-utils@11.0.1
- react-scripts@4.0.1
Co-authored-by: Ryota Murakami <dojce1048@gmail.com>
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
Co-authored-by: Ian Sutherland <ian@iansutherland.ca>
Co-authored-by: sho90 <aznecosann@gmail.com>
Co-authored-by: Anyul Rivas <anyulled@gmail.com>
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Co-authored-by: Jeffrey Posnick <jeffy@google.com>
Co-authored-by: Evan Bacon <baconbrix@gmail.com>
blackarctic added a commit to blackarctic/create-react-app that referenced this pull request Apr 29, 2021
* Fix noFallthroughCasesInSwitch/jsx object is not extensible (react#9921)
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
* Add logo license to README
* Remove trailing space in reportWebVitals.ts (react#10040)
* docs: add React Testing Library as a library requiring jsdom (react#10052)
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
* Increase Workbox's maximumFileSizeToCacheInBytes (react#10048)
* Create FUNDING.yml
* replace inquirer with prompts (react#10083)
- remove `react-dev-utils/inquirer` public import
* Prepare 4.0.1 release
* Prepare 4.0.1 release
* Publish
- cra-template-typescript@1.1.1
- cra-template@1.1.1
- create-react-app@4.0.1
- react-dev-utils@11.0.1
- react-scripts@4.0.1
* chore: bump web-vital dependency version (react#10143)
* chore: bump typescript version (react#10141)
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
* Add TypeScript 4.x as peerDependency to react-scripts(react#9964)
* remove chalk from formatWebpackMessages (react#10198)
* Upgrade @svgr/webpack to fix build error (react#10213)
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
* Improve vendor chunk names in development (react#9569)
* Update postcss packages (react#10003)
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
* Recovered some integration tests (react#10091)
* Upgrade sass-loader (react#9988)
* Move ESLint cache file into node_modules (react#9977)
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
* Revert "Update postcss packages" (react#10216)
This reverts commit 580ed5d.
* Remove references to Node 8 (react#10214)
* fix(react-scripts): add missing peer dependency react and update react-refresh-webpack-plugin (react#9872)
* Update using-the-public-folder.md (react#10314)
Some library --> Some libraries
* docs: add missing override options for Jest config (react#9473)
* Fix CI tests (react#10217)
* appTsConfig immutability handling by immer (react#10027)
Co-authored-by: mad-jose <joset@yeswearemad.com>
* Add support for new BUILD_PATH advanced configuration variable (react#8986)
* Add opt-out for eslint-webpack-plugin (react#10170)
* Prepare 4.0.2 release
* Publish
- cra-template-typescript@1.1.2
- cra-template@1.1.2
- create-react-app@4.0.2
- react-dev-utils@11.0.2
- react-error-overlay@6.0.9
- react-scripts@4.0.2
* tests: update test case to match the description (react#10384)
* Bump webpack-dev-server 3.11.0 -> 3.11.1 (react#10312)
Resolvesreact#10084 security vulnerability in websocket-driver library version 0.5.6, imported transitively by sockjs
* Upgrade eslint-webpack-plugin to fix opt-out flag (react#10590)
* update immer to 8.0.1 to address vulnerability (react#10412)
Resolvesreact#10411
Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.
* Prepare 4.0.3 release
* Update CHANGELOG
* Publish
- create-react-app@4.0.3
- react-dev-utils@11.0.3
- react-scripts@4.0.3
Co-authored-by: Ryota Murakami <dojce1048@gmail.com>
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
Co-authored-by: Ian Sutherland <ian@iansutherland.ca>
Co-authored-by: sho90 <aznecosann@gmail.com>
Co-authored-by: Anyul Rivas <anyulled@gmail.com>
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Co-authored-by: Jeffrey Posnick <jeffy@google.com>
Co-authored-by: Evan Bacon <baconbrix@gmail.com>
Co-authored-by: Sahil Purav <sahil5684@gmail.com>
Co-authored-by: Hakjoon Sim <trainto@gmail.com>
Co-authored-by: Chris Shepherd <chris@chrisshepherd.me>
Co-authored-by: Jason Williams <936006+jasonwilliams@users.noreply.github.com>
Co-authored-by: Jabran Rafique⚡️ <jabranr@users.noreply.github.com>
Co-authored-by: John Ruble <johnruble@gmail.com>
Co-authored-by: Morten N.O. Nørgaard Henriksen <morten.n.o.henriksen@icloud.com>
Co-authored-by: Sergey Makarov <serega.s.makar@gmail.com>
Co-authored-by: EhsanKhaki <ehsankhfr@gmail.com>
Co-authored-by: Kristoffer K <merceyz@users.noreply.github.com>
Co-authored-by: Aviv Hadar <Avivhdr@gmail.com>
Co-authored-by: Tobias Büschel <13087421+tobiasbueschel@users.noreply.github.com>
Co-authored-by: mad-jose <44253495+josezone@users.noreply.github.com>
Co-authored-by: mad-jose <joset@yeswearemad.com>
Co-authored-by: Andrew Hyndman <ajhyndman@hotmail.com>
Co-authored-by: Brody McKee <mrmckeb@users.noreply.github.com>
Co-authored-by: James George <jamesgeorge998001@gmail.com>
Co-authored-by: Dion Woolley <woolley.dion@gmail.com>
Co-authored-by: Walker Clem <51654951+wclem4@users.noreply.github.com>
wombleton pushed a commit to AurorNZ/create-react-app that referenced this pull request Jun 1, 2021
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
abhiisheek pushed a commit to abhiisheek/create-react-app that referenced this pull request May 19, 2023
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
abhiisheek pushed a commit to abhiisheek/create-react-app that referenced this pull request May 24, 2023
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@ryota-murakami@mrmckeb@benneq@KonstantinSimeonov@ianschmitz@vegawong@cseas@facebook-github-bot