Skip to content

Conversation

@ianyong
Copy link
Member

@ianyong ianyong commented Sep 3, 2023

Description

Node.js APIs are only available in Node.js environments (and not browser environments). In order to make use of Node.js APIs in the browser, Webpack includes polyfills of these APIs (libraries that contain the same or similar API but are implemented in pure JavaScript so that they can be run in the browser).

Webpack appears to polyfill the path module by default, but not the path/posix module. This PR adds the path-browserify library as a polyfill for path/posix. Note that the lock file does not change because Webpack already depends on path-browserify.

Fixes the build issue seen in #2655.

Relevant js-slang PR: source-academy/js-slang#1470

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

  1. Bump js-slang to v1.0.34.
  2. Run yarn run build and verify that there are no errors.

Checklist

  • I have tested this code
  • I have updated the documentation

It is currently an indirect dependency of Webpack, but since we
want to reference it in our code, we should declare it explicitly.
@ianyong ianyong requested a review from martin-henz September 3, 2023 11:59
@RichDom2185 RichDom2185 self-requested a review September 3, 2023 12:05
Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick fix!

@RichDom2185 RichDom2185 merged commit 3413686 into master Sep 3, 2023
@RichDom2185 RichDom2185 deleted the polyfill-path-posix branch September 3, 2023 12:06
RichDom2185 pushed a commit to NUS-CS1101S/cadet-frontend that referenced this pull request Sep 10, 2023
* Specify `path-browserify` as a direct dev dependency

It is currently an indirect dependency of Webpack, but since we
want to reference it in our code, we should declare it explicitly.

* Polyfill `path/posix` library when building for browsers
RichDom2185 pushed a commit to NUS-CS1101S/cadet-frontend that referenced this pull request Sep 10, 2023
* Specify `path-browserify` as a direct dev dependency

It is currently an indirect dependency of Webpack, but since we
want to reference it in our code, we should declare it explicitly.

* Polyfill `path/posix` library when building for browsers
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

Successfully merging this pull request may close these issues.

3 participants