-
Notifications
You must be signed in to change notification settings - Fork 0
Enhance XMLHttpRequest docs and implementation #17
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, only direct `xhr.onreadystatechange` was mocked, which means `xhr.addEventListener('readystatechange', () => {...})` wouldn't be mocked. To fix this: * Add new `createEvent()` util function to create actual DOM events of the specified type, attaches them to the desired element/object, and fires them on command. * Abstract the `readystatechange` mocking logic out to a separate array and function. * Iterate through all specified events to mock out both `xhr.onX` and `xhr.addEventListener('X')` so they both receive the mocked response. * Still wait for `delay` milliseconds before firing the two.
It's another handler/event commonly used to check the status of an XHR request since it helps show progress of the request instead of just a boolean of whether or not it's finished. It's also what Axios now prefers over onreadystatechange. See [this GitHub issue](#16) for more details.
It doesn't exist in jest DOM and most resources online say there's not really a good polyfill for it/we need to mock it out ourselves
It's another event commonly used. It's also what Axios now prefers over onreadystatechange.
TODO Upgrade Babel configuration to support optional chaining
This allows the functions to be called with no arguments but still work as expected
Also, remove superfluous `console.log()` used during development
…le = true` Per MDN docs, `lengthComputable` should be true if it's possible to calculate the length of the response. Since the mock response is predetermined by the user, we can in fact calculate the length. Thus, set it to true. See: https://developer.mozilla.org/en-US/docs/Web/API/ProgressEvent/lengthComputable
… proceeding While no difference was seen in tests' success/failure, and even though `xhr.send()` is not async, MockRequests makes it async which is perfect for unit tests. Thus, ensure the XHR tests always wait for the mock-network-call promise to resolve to harden tests against false positives.
Activate mocks before each test and restore them to their original (non-mocked) values after each test. This ensures any `jest.spyOn()` calls made within individual tests don't affect subsequent tests.
…eEvent()` are defined Also, add tests for the mock Event object
…ation inside the functions
GitHub supports custom `<a name=custom-href />` usage, but npmjs.com doesn't. Thus, make table of contents work there by making hyperlinks refer to actual heading names. Also, update headings' text and/or nesting structure for better traversal UX.
…nce more apparent
Helps with other third-party libraries that mock XHR and need `headers` to be defined
…r `node-fetch` Also, add the same note that MockRequests needs the network functions defined globally in the "Implementation notes" section
…te MJS/CJS examples
This allows them to modify the values as they wish, including overwriting `writable`/`value` with getters/setters
`loadend` is called regardless of success/failure whereas `load` is only called upon success. Support them both because users may want separate handlers for success/failure or they may want to use the same handler for both (e.g. Axios).
…Axios to function
…se of understanding
It is recommended to use NodeJS >= v14, but this app was last built with NodeJS v10. Thus, update both the app and demo with NodeJS v16. In the process, add typedefs for `jest` and `node` for better IDE autocompletion and replace the deprecated `node-sass` with `sass`. Note: The app and demo both still run perfectly on IE 11.
…ls run before src code The runtime file is generally used to map different chunks/files to numbers for `__webpack_require__[num]`. By default, this logic is spread across entry files rather than in its own file. Separating it out to its own file helps to ensure all entries' associated build output files know what import maps to what which code at all times. This is particularly important with regards to the built index.html. HtmlWebpackPlugin no longer supports `chunksSortMode: 'dependency'`; this means that, by default, it adds JS <script> tags in the order they're listed in `webpackConfig.entry` rather than dependency-hierarchy (see: jantimon/html-webpack-plugin@22fb03f). And because it was removed, re-implementing it manually would require either (1) manually declaring chunk order (not DRY and easy to forget when adding a new entry), (2) a custom wrapper plugin around HtmlWebpackPlugin to dynamically scan through the `entry` options and order the `hwpOptions.chunks` array accordingly, or (3) the same logic as (2) but executed outside of the `webpackConfig` object itself (requiring either `entry` or HWP to also be separate from the final config object so that they can be read/written to accordingly). All that to say that without a separate runtime file, it's possible that the scripts will begin executing out-of-order. Due to the usage of `__webpack_require__`, this isn't an issue for imported code, but it is an issue for non-imported code, i.e. polyfills. Thus, splitting the runtime mapping logic out to a separate file ensures that even though <script> tags are executed out-of-order, one script won't run without its dependency running first. (Conceptually, you can think of it as `runtime.js` being the only function that is called on page load, and all other entry point scripts just being functions waiting to be called by `runtime.js`.)
Ensure it starts on the ReadMe page and not a sub-page
This was referenced Nov 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes issues #14, #15, and #16.
Primary changes:
onload
andonloadend
events in addition toreadystatechange
in order to supportaxios@>=0.22.2
.on[event]
andaddEventListener(event)
methods forXMLHttpRequest
.axios
withXMLHttpRequest
polyfills.Secondary changes:
node@>=14
for better dependency resolution.node-sass
withsass
and fixing the incorrect re-defining ofvendor
in bothentry
andoptimization
within webpack.config.js.setupFiles
tosetupFilesAfterEnv
.<a name="X"/>
).addEventListener()
usages.