Expose all built-in options for `getConstructor` by hibachrach · Pull Request #1050 · reactjs/react-rails

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach · 2020-01-10T08:15:18Z

Summary

This enables us to easily override getConstructor to not use global
fallback, avoiding the all-consuming try...catch.

Other Information

Here's the context:

Quoting #264 (comment)

Regarding Encountered error "#<ExecJS::ProgramError: Invariant Violation: Element type is invalid: ...:
I think one of the core issues is that module lookup uses
try...catch.
While the errors are logged to the console shim, that typically doesn't
help as a later error (such as the invariant violation) will lead to a
fatal error (triggering a 500). If that could be refactored to be a bit
more intentional based on environment (instead of just reacting based on
exceptions, or at the very least, throwing if the caught exception isn't
very specific)

justin808 · 2022-08-17T10:32:58Z

@HarrisonB can you rebase this on master?

hibachrach · 2022-08-17T22:55:11Z

Sure! I'll hopefully get to it in the next week or two

hibachrach · 2022-08-26T04:32:07Z

@justin808 updated!

justin808 · 2022-08-26T05:49:10Z

@HarrisonB can you confirm that all unit tests run locally?

Currently, we don't have CI working. We need to move to GH Actions.

hibachrach · 2022-08-30T02:06:22Z

Having some trouble--See #1198 (comment)

justin808 · 2022-09-27T10:04:07Z

@hibachrach is this ready for merge?

hibachrach · 2022-09-29T21:54:32Z

yes, though we should have it trigger CI to make sure it passes. is there a way to do that on your end?

justin808 · 2022-09-30T09:48:54Z

@hibachrach did you rebase on master where the github actions are defined?

Quoting reactjs#264 (comment) > Regarding `Encountered error "#<ExecJS::ProgramError: Invariant > Violation: Element type is invalid: ...`: > > I think one of the core issues is that [module lookup uses > `try...catch`](https://github.com/reactjs/react-rails/blob/master/react_ujs/src/getConstructor/fromRequireContextWithGlobalFallback.js#L11-L23). > While the errors are logged to the console shim, that typically doesn't > help as a later error (such as the invariant violation) will lead to a > fatal error (triggering a 500). If that could be refactored to be a bit > more intentional based on environment (instead of just reacting based on > exceptions, or at the very least, throwing if the caught exception isn't > very specific) This enables us to easily override `getConstructor` to not use global fallback, avoiding the all-consuming `try...catch`.

hibachrach · 2022-10-01T06:15:44Z

Tests are now passing!

alkesh26 · 2022-10-07T11:59:25Z

@justin808 The PR is good to go if we have no review comments.

justin808 · 2022-10-19T19:53:36Z

Thanks @hibachrach!

hibachrach marked this pull request as ready for review January 10, 2020 21:04

hibachrach force-pushed the add-additional-options-for-getConstructor branch from ee7a0f4 to 332667dCompare August 18, 2022 04:25

hibachrach force-pushed the add-additional-options-for-getConstructor branch from 332667d to 9aabc79Compare September 30, 2022 15:16

justin808 merged commit 4cbf13d into reactjs:masterOct 19, 2022

hibachrach deleted the add-additional-options-for-getConstructor branch October 19, 2022 23:40