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

#22988 - Fix Bug: @license header in React 18 bundles contains vundefined #23004

Merged
merged 2 commits into from
Dec 21, 2021

Conversation

vitaliemiron
Copy link
Contributor

Fix the version issue - #22988

@salazarm
Copy link
Contributor

try
reactVersion = require('../../packages/shared/ReactVersion')

@vitaliemiron
Copy link
Contributor Author

try reactVersion = require('../../packages/shared/ReactVersion')

doesn't work for me, do you have any idea why ?
Screenshot 2021-12-21 at 18 22 03

@bvaughn bvaughn self-requested a review December 21, 2021 16:43
Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Thanks!

Actually, I need to think on this one further.

This change will fix the undefined issue but the version number shown in the header comment will lag behind the version we are building to publish.

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Removing approval for now to avoid confusion.

@vitaliemiron
Copy link
Contributor Author

If I can help with something else LMK please

@bvaughn
Copy link
Contributor

bvaughn commented Dec 21, 2021

There's an interesting mix of concerns here between the Rollup scripts that are building the JS bundles we're about to publish and the bespoke release scripts we use that inject the right version number after building and before publishing.

Honestly maybe the best fix here for now is just to remove the version from the header comment. 😄

diff --git a/scripts/rollup/wrappers.js b/scripts/rollup/wrappers.js
index 46b162cee..9f61b8b17 100644
--- a/scripts/rollup/wrappers.js
+++ b/scripts/rollup/wrappers.js
@@ -3,7 +3,6 @@
 const {resolve} = require('path');
 const {readFileSync} = require('fs');
 const {bundleTypes, moduleTypes} = require('./bundles');
-const reactVersion = require('../../packages/react-dom/package.json').version;
 
 const {
   NODE_ES2015,
@@ -54,7 +53,8 @@ const license = ` * Copyright (c) Facebook, Inc. and its affiliates.
 const wrappers = {
   /***************** NODE_ES2015 *****************/
   [NODE_ES2015](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -67,7 +67,8 @@ ${source}`;
 
   /***************** NODE_ESM *****************/
   [NODE_ESM](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -78,7 +79,8 @@ ${source}`;
 
   /***************** UMD_DEV *****************/
   [UMD_DEV](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -88,7 +90,8 @@ ${source}`;
 
   /***************** UMD_PROD *****************/
   [UMD_PROD](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -98,7 +101,8 @@ ${license}
 
   /***************** UMD_PROFILING *****************/
   [UMD_PROFILING](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -108,7 +112,8 @@ ${license}
 
   /***************** NODE_DEV *****************/
   [NODE_DEV](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -125,7 +130,8 @@ ${source}
 
   /***************** NODE_PROD *****************/
   [NODE_PROD](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -135,7 +141,8 @@ ${source}`;
 
   /***************** NODE_PROFILING *****************/
   [NODE_PROFILING](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -294,7 +301,8 @@ ${source}`;
 const reconcilerWrappers = {
   /***************** NODE_DEV (reconciler only) *****************/
   [NODE_DEV](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -313,7 +321,8 @@ ${source}
 
   /***************** NODE_PROD (reconciler only) *****************/
   [NODE_PROD](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}
@@ -327,7 +336,8 @@ ${source}
 
   /***************** NODE_PROFILING (reconciler only) *****************/
   [NODE_PROFILING](source, globalName, filename, moduleType) {
-    return `/** @license React v${reactVersion}
+    return `/**
+ * @license React
  * ${filename}
  *
 ${license}

@vitaliemiron
Copy link
Contributor Author

vitaliemiron commented Dec 21, 2021

haha, ok, i pushed the change

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Thanks!

@bvaughn bvaughn merged commit bd0a5dd into facebook:main Dec 21, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jan 10, 2022
Summary:
This sync includes the following changes:
- **[fe905f152](facebook/react@fe905f152 )**: Update package.json ([#22954](facebook/react#22954)) //<Jack Works>//
- **[3dc41d8a2](facebook/react@3dc41d8a2 )**: fix: parseExportNamesInto specifiers typo ([#22537](facebook/react#22537)) //<btea>//
- **[bd0a5dd68](facebook/react@bd0a5dd68 )**: #22988 - Fix Bug: license header in React 18 bundles contains vundefined ([#23004](facebook/react#23004)) //<Vitalie>//
- **[ceee524a8](facebook/react@ceee524a8 )**: Remove unnecessary clearContainer call ([#22979](facebook/react#22979)) //<Sebastian Markbåge>//
- **[cd1a3e9b5](facebook/react@cd1a3e9b5 )**: Build both a partial renderer and fizz renderer of the legacy API for FB ([#22933](facebook/react#22933)) //<Sebastian Markbåge>//

Changelog:
[General][Changed] - React Native sync for revisions a049aa0...fe905f1

jest_e2e[run_all_tests]

Reviewed By: rickhanlonii

Differential Revision: D33512179

fbshipit-source-id: c2df06c8af6bb674ea0c5524538259e6d6d98f78
@gaearon gaearon mentioned this pull request Mar 29, 2022
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
…s vundefined (facebook#23004)

* Fix Bug: @license header in React 18 bundles contains vundefined
* Remove React version from the header comment
nevilm-lt pushed a commit to nevilm-lt/react that referenced this pull request Apr 22, 2022
…s vundefined (facebook#23004)

* Fix Bug: @license header in React 18 bundles contains vundefined
* Remove React version from the header comment
Sign up for free to 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.

4 participants