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

⬇ Error in the date selector test + πŸš‘ WSOD in prod build #1113

Closed
shankari opened this issue Feb 16, 2025 · 16 comments
Closed

⬇ Error in the date selector test + πŸš‘ WSOD in prod build #1113

shankari opened this issue Feb 16, 2025 · 16 comments

Comments

@shankari
Copy link
Contributor

The jest tests for the date selector have been failing for the past week. None of the subsequent changes are related to date selection, so I suspect this is due to a change in the version.

Image
@shankari
Copy link
Contributor Author

Fortunately, on successful runs, we upload the exact versions using package-lock.json, and the archives from the previous successful run (two months ago!) are still available.

Image

@shankari
Copy link
Contributor Author

The diffs are fairly significant and at least at first glance, don't seem to affect the animate libraries that are in the stack trace.

Summary of all failing tests
 FAIL  www/__tests__/DateSelect.test.tsx (29.342 s)
  ● DateSelect β€Ί renders correctly

    TypeError: Cannot read properties of undefined (reading '__isNative')

       7 | }));
       8 | jest.spyOn(React, 'useState').mockImplementation((initialValue) => [initialValue, jest.fn()]);
    >  9 | jest.spyOn(React, 'useEffect').mockImplementation((effect: () => void) => effect());
         |                                                                           ^
      10 |
      11 | describe('DateSelect', () => {
      12 |   it('renders correctly', () => {

      at __isNative (node_modules/react-native/Libraries/Animated/useAnimatedProps.js:264:14)
      at effect (www/__tests__/DateSelect.test.tsx:9:75)
      at useAnimatedPropsLifecycle_layoutEffects (node_modules/react-native/Libraries/Animated/useAnimatedProps.js:258:12)
      at useAnimatedPropsLifecycle (node_modules/react-native/Libraries/Animated/useAnimatedProps.js:82:3)
      at node_modules/react-native/Libraries/Animated/createAnimatedComponent.js:65:57
      at renderWithHooks (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6351:18)

So it is not obvious what to revert.

@shankari
Copy link
Contributor Author

However, if I copy over the package-lock.json into a fresh copy of the phone repo, the setup process installs the exact versions and the test works

$ git clone https://github.com/e-mission/e-mission-phone.git
$ cd e-mission-phone
$ cp ../package-lock.json .
$ bash setup/setup_serve.sh
$ source setup/activate_shared.sh
$ npx run test

results in

Test Suites: 27 passed, 27 total
Tests:       182 passed, 182 total
Snapshots:   0 total
Time:        10.465 s
Ran all test suites.

@shankari
Copy link
Contributor Author

shankari commented Feb 16, 2025

Note further that per the npm documentation, https://docs.npmjs.com/cli/v10/configuring-npm/package-lock-json?v=true

This file is intended to be committed into source repositories, and serves various purposes:
Describe a single representation of a dependency tree such that teammates, deployments, and continuous integration are guaranteed to install exactly the same dependencies.

So it sounds like maybe we should check this working version into the source code. I know that we tried to do that before, but then (probably due to an older version of npm), every run of npm install would override package-lock.json. So I removed it because there were a lot of changes that were not technically intentional.

But with this npm behavior, maybe we should re-think that, and check in the package-lock.serve.json and package-lock.cordovabuild.json as well even for the long-term.

shankari added a commit to e-mission/e-mission-phone that referenced this issue Feb 16, 2025
So we can have the CI use known good versions of the dependencies.
Without this change, the underlying libraries can change based on the loosely
defined semver in the `package.json`, which can cause unexpected failures.

An example of such a failure is:
e-mission/e-mission-docs#1113

Copying over the `package-lock.json` overrides `package.json` and forces the build to use the exact versions specified
e-mission/e-mission-docs#1113 (comment)

Note that `package-lock.json` is apparently designed to checked in to source
control. It used to be checked in (thanks to a contribution from the UNSW team)
but I removed it because every run of `npm install` would override it,
resulting in confusing merge conflicts.

However, it looks like `npm` has now changed its behavior so it respects the
`package-lock.json` if it exists.
e-mission/e-mission-docs#1113 (comment)

So we restore it by copying in known good versions - from
https://github.com/e-mission/e-mission-phone/actions/runs/12448538357
for the "serve" case and from the last successful internal build of NREL
OpenPATH for the `cordovabuild` case.

We also change `bin/configure_xml_and_json.js` to copy these files over as well

Testing done:
- Copying `package-lock.json` into the repo causes `npm install` to work, and
  the tests to pass
- Testing of the setup changes will happen through CI
shankari added a commit to e-mission/e-mission-phone that referenced this issue Feb 16, 2025
So we can have the CI use known good versions of the dependencies.
Without this change, the underlying libraries can change based on the loosely
defined semver in the `package.json`, which can cause unexpected failures.

An example of such a failure is:
e-mission/e-mission-docs#1113

Copying over the `package-lock.json` overrides `package.json` and forces the build to use the exact versions specified
e-mission/e-mission-docs#1113 (comment)

Note that `package-lock.json` is apparently designed to checked in to source
control. It used to be checked in (thanks to a contribution from the UNSW team)
but I removed it because every run of `npm install` would override it,
resulting in confusing merge conflicts.

However, it looks like `npm` has now changed its behavior so it respects the
`package-lock.json` if it exists.
e-mission/e-mission-docs#1113 (comment)

So we restore it by copying in known good versions - from
https://github.com/e-mission/e-mission-phone/actions/runs/12448538357
for the "serve" case and from the last successful internal build of NREL
OpenPATH for the `cordovabuild` case.

We also change `bin/configure_xml_and_json.js` to copy these files over as well

Testing done:
- Copying `package-lock.json` into the repo causes `npm install` to work, and
  the tests to pass
- Testing of the setup changes will happen through CI
@shankari
Copy link
Contributor Author

Note that since I copied over the package-lock.cordovabuild.json from the last build of NREL OpenPATH, it does not include 157afeca40ec9971e68daa2a19a22daab0d8e844

@shankari
Copy link
Contributor Author

And the native build fails with the error.

$ npm run build-dev
    
> gov.nrel.cims.openpath@1.9.7 build-dev
> npx webpack --config webpack.dev.js && npx cordova build
    
node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^
    
Error: Cannot find package '@react-native/babel-preset' imported from /openpath-phone/babel-virtual-resolve-base.js 
    at new NodeError (/openpath-phone/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:194:5)
    at packageResolve (/openpath-phone/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:908:9)
    at moduleResolve (/openpath-phone/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:937:20)
    at defaultResolve (/openpath-phone/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:1007:15)   
    at resolve (/openpath-phone/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:1020:12)
    at tryImportMetaResolve (/openpath-phone/node_modules/@babel/core/lib/config/files/plugins.js:142:45)
    at resolveStandardizedNameForImport (/openpath-phone/node_modules/@babel/core/lib/config/files/plugins.js:164:19)
    at resolveStandardizedName (/openpath-phone/node_modules/@babel/core/lib/config/files/plugins.js:173:22) 
    at loadPreset (/openpath-phone/node_modules/@babel/core/lib/config/files/plugins.js:61:20)
    at loadPreset.next (<anonymous>)
    at createDescriptor (/openpath-phone/node_modules/@babel/core/lib/config/config-descriptors.js:140:16)   

    at step (/openpath-phone/node_modules/gensync/index.js:269:25)
    at evaluateAsync (/openpath-phone/node_modules/gensync/index.js:291:5)
    at /openpath-phone/node_modules/gensync/index.js:93:9
    at new Promise (<anonymous>)
    at async (/openpath-phone/node_modules/gensync/index.js:92:14)
    at stopHiding - secret - don't use this - v1 (/openpath-phone/node_modules/@babel/core/lib/errors/rewrite-stack-trace.js:47:12) 
    at Object.loadPartialConfigAsync (/openpath-phone/node_modules/@babel/core/lib/config/index.js:34:85)
    at Object.loader (/openpath-phone/node_modules/babel-loader/lib/index.js:109:30)
    at Object.<anonymous> (/openpath-phone/node_modules/babel-loader/lib/index.js:39:12)
    at LOADER_EXECUTION (/openpath-phone/node_modules/loader-runner/lib/LoaderRunner.js:132:14)
    at runSyncOrAsync (/openpath-phone/node_modules/loader-runner/lib/LoaderRunner.js:133:4)
    at iterateNormalLoaders (/openpath-phone/node_modules/loader-runner/lib/LoaderRunner.js:251:2)
    at /openpath-phone/node_modules/loader-runner/lib/LoaderRunner.js:224:4
    at /openpath-phone/node_modules/webpack/lib/NormalModule.js:840:15
    at Array.eval (eval at create (/openpath-phone/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:12:1)
    at runCallbacks (/openpath-phone/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:43:15)
    at /openpath-phone/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:249:5
    at /openpath-phone/node_modules/graceful-fs/graceful-fs.js:123:16
    at FSReqCallback.readFileAfterClose [as oncomplete] (node:internal/fs/read/context:68:3) {
  code: 'ERR_MODULE_NOT_FOUND'
}   
  code: 'ERR_MODULE_NOT_FOUND'
}

The weird thing is that the build succeeded for my pinning change. So I am not quite sure why it failed for OpenPATH.

Image

@shankari
Copy link
Contributor Author

Ah the CI build uses npx cordova build android and not npm run build, which means that it doesn't actually rebuild the UI elements. Not sure if we should change that; I think that the assumption was that the serve would check the javascript parts and the android and iOS parts would check the native build.

But given that the package.json is so different on serve and cordovabuild, variations between the two package files can break the build; need to figure out how to resolve this.

@shankari
Copy link
Contributor Author

I'm getting the same error even after removing package-lock.json completely and running npm install so it is regenerated.
I didn't make the babel changes, so I am going to pause this descent into dependency hell for now...

@shankari
Copy link
Contributor Author

Ah this is because of my fix from Dec 21st.
e-mission/e-mission-phone#1193

That was the day of the last release, so this is the first time I am building with this change. Not sure why npm install isn't installing the appropriate module to get babel to work...

@shankari
Copy link
Contributor Author

$ npm i @react-native/babel-preset gets past that error, but I am not sure why it wasn't needed in the package.serve.json and how that automagically installed the module. I'm now running into an issue with a missing icon, but that doesn't seem related to this.

shankari added a commit to e-mission/e-mission-phone that referenced this issue Feb 18, 2025
This was originally added in 4566cc0
and seems to work fine for the `serve` case

But it doesn't work for the `cordovabuild` case
e-mission/e-mission-docs#1113 (comment)
e-mission/e-mission-docs#1113 (comment)

Adding it, although I still don't know why the `serve` case did not need it
@shankari shankari changed the title ⬇ Error in the date selector test ⬇ Error in the date selector test + πŸš‘ WSOD in prod build Feb 19, 2025
@shankari
Copy link
Contributor Author

As a next issue, although the dev build works well, the prod build generates a WSOD.
Checking the logs, we see that the error is

Uncaught ReferenceError: require is not defined
    at 79292 (bundle.js:2:3753132)
    at __webpack_require__ (bundle.js:2:5102691)
    at 6623 (bundle.js:2:9430)
    at __webpack_require__ (bundle.js:2:5102691)
    at bundle.js:2:5106774
    at bundle.js:2:5106801

Since it happens in the prod build and not in the dev build, it is likely due to transpiling and is probably due to
e-mission/e-mission-phone#1193

and our use of RN web instead of RN
https://stackoverflow.com/questions/45818937/webpack-uncaught-referenceerror-require-is-not-defined

Since the dev build works, and we added the additional transpiling to get unit tests (that run against the dev build) to work, I will try to fix by enabling that preset only for dev builds
https://babeljs.io/docs/configuration#javascript-configuration-files

@shankari
Copy link
Contributor Author

I can confirm manually removing that preset removes the WSOD on the release build. I am pushing out a new release with this change right now, and will fix in the codebase next.

@shankari
Copy link
Contributor Author

shankari commented Feb 20, 2025

Changing babel.config.js to match the standard format

module.exports = function(api) {
    api.cache(true);
    const presets = ['@babel/preset-env',
        '@babel/preset-typescript',
        '@babel/preset-react',
        'module:@react-native/babel-preset'];
    const plugins = ['@babel/plugin-transform-flow-strip-types'];

    return {presets, plugins};
}

works

Now let's make it dependent on the environment.

@shankari
Copy link
Contributor Author

However, adding the log console.log("Running in environment "+api.env()); fails with

 FAIL  www/__tests__/storeDeviceSettings.test.ts
  ● Test suite failed to run

    Caching has already been configured with .never or .forever()

       7 |     const plugins = ['@babel/plugin-transform-flow-strip-types'];
       8 |
    >  9 |     console.log("Running in environment "+api.env());
         |                                               ^
      10 |
      11 |     return {presets, plugins};
      12 | }

      at CacheConfigurator.using (node_modules/@babel/core/lib/config/caching.js:188:13)
      at Object.env (node_modules/@babel/core/lib/config/helpers/config-api.js:19:30)
      at module.exports (babel.config.js:9:47)
      at loadPartialConfigSync (node_modules/@babel/core/lib/config/index.js:37:84)
      at loadPartialConfig (node_modules/@babel/core/lib/config/index.js:46:14)
      at ScriptTransformer._getCacheKey (node_modules/@jest/transform/build/ScriptTransformer.js:228:41)
      at ScriptTransformer._getFileCachePath (node_modules/@jest/transform/build/ScriptTransformer.js:289:27)
      at ScriptTransformer.transformSource (node_modules/@jest/transform/build/ScriptTransformer.js:525:32)
      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:674:40)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:726:19)

@shankari
Copy link
Contributor Author

Using process.env.ENV works, although I don't see the log output.

However, trying to use if (process.env.ENV === "development") { does not trigger adding the new module. We continue to get errors such as

    SyntaxError: /private/tmp/e-mission-phone-working/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js: Unexpected token, expected "]" (39:5)

which are consistent with the 'module:@react-native/babel-preset' not being present.

I guess it may be because when testing, the environment is not development. I am not sure what it actually is, but flipping the check should get it to work.

if (process.env.ENV !== "production") { does work!

@shankari
Copy link
Contributor Author

Remaining cleanups for this task are:

  • Figure out whether we should commit the package-lock file(s) and what the process for updating them should be
  • Debug the date selector test so that we can update the package-lock.serve.json to the most recent value
  • figure out what the process.env.ENV for testing is and potentially change the check to use it directly

shankari added a commit to e-mission/e-mission-phone that referenced this issue Feb 20, 2025
In the previous commit, 1089679
we copied over the cordovabuild package-lock from the internal build of NREL
OpenPATH.

However, when trying to get the internal build to complete, for this release, I
moved out the existing package-lock and regenerated, as an attempt to fix
e-mission/e-mission-docs#1113 (comment)

After adding the new module (in b9e9a37)
all of the dependency versions also changed. Since `package-lock.json` is
supposed to represent the checked in "time travel" version of the modules
actually used, we update this to reflect the current module versions used as
well.
shankari added a commit to e-mission/e-mission-phone that referenced this issue Feb 20, 2025
In #1193, I added
`module:@react-native/babel-preset`

That caused the unit tests to work, but caused a WSOD with the release build
e-mission/e-mission-docs#1113 (comment)

So we exclude the preset for production builds

I tried to add it only to dev builds, but the `process.env.ENV` is not set correctly
so I had to switch the condition
e-mission/e-mission-docs#1113 (comment)

Testing done:

```
$ npm run test

Test Suites: 27 passed, 27 total
Tests:       182 passed, 182 total
Snapshots:   0 total
Time:        10.537 s
Ran all test suites.
```
shankari added a commit to e-mission/e-mission-phone that referenced this issue Feb 20, 2025
In #1193, I added
`module:@react-native/babel-preset`

That caused the unit tests to work, but caused a WSOD with the release build
e-mission/e-mission-docs#1113 (comment)

So we exclude the preset for production builds

I tried to add it only to dev builds, but the `process.env.ENV` is not set correctly
so I had to switch the condition
e-mission/e-mission-docs#1113 (comment)

Testing done:

```
$ npm run test

Test Suites: 27 passed, 27 total
Tests:       182 passed, 182 total
Snapshots:   0 total
Time:        10.537 s
Ran all test suites.
```
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

No branches or pull requests

1 participant