8000 Support installed npm modules and relative require by jclem · Pull Request #135 · actions/github-script · GitHub
[go: up one dir, main page]
Skip to content

Support installed npm modules and relative require #135

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.

Sign up for GitHub

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
merged 25 commits into from
Apr 21, 2021
Merged

Support installed npm modules and relative require #135

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ddba1b1
Use non-Webpack-require in evaluated scripts
jclem Apr 21, 2021
f4e5d39
Use a proxy to support relative requires
jclem Apr 21, 2021
e853490
Run build script
jclem Apr 21, 2021
75e3a5b
Move wrapRequire to its own module
jclem Apr 21, 2021
c416f56
Pass nativeRequire, as well
jclem Apr 21, 2021
83c92d6
Rename nativeRequire to __original_require__
jclem Apr 21, 2021
3ca4cd5
Update documentation for relative require support
jclem Apr 21, 2021
c1c139b
Add a catch to support requiring installed modules
jclem Apr 21, 2021
ade5cea
Use concat for paths
jclem Apr 21, 2021
f57c84a
Try global.module
jclem Apr 21, 2021
1f8fc98
Run build
jclem Apr 21, 2021
d37f92f
Test eval
jclem Apr 21, 2021
7e8659d
Run build
jclem Apr 21, 2021
7e12bd7
Call resolve, then require
8000 jclem Apr 21, 2021
b0e12e7
Add Webpack comment vis-a-vis eval
jclem Apr 21, 2021
01f87b6
Remove caveat about node_modules
jclem Apr 21, 2021
3ede589
Add new integration tests
jclem Apr 21, 2021
19e7914
Add integration test running
jclem Apr 21, 2021
4a93ad9
Add actions/checkout
jclem Apr 21, 2021
fb7e8f7
Add npm ci to integration test
jclem Apr 21, 2021
5ee517d
Workflow syntax error
jclem Apr 21, 2021
256da4e
Update src/wrap-require.ts
jclem Apr 21, 2021
c758586
v4.0.0
jclem Apr 21, 2021
b616178
Merge branch 'wrap-require' of https://github.com/actions/github-scri…
jclem Apr 21, 2021
2923e50
Run build script
jclem Apr 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add Webpack comment vis-a-vis eval
  • Loading branch information
@jclem
jclem committed Apr 21, 2021
commit b0e12e725ba5182c4b0cb8c2bf2ca0f25097754b
2 changes: 2 additions & 0 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2906,6 +2906,8 @@ const wrapRequire = new Proxy(require, {
const modulePath = target.resolve.apply(thisArg, [
moduleID,
{
// Webpack does not have an escape hatch for getting the actual
// module, other than `eval`.
paths: eval('module').paths.concat(process.cwd())
}
]);
Expand Down
2 changes: 2 additions & 0 deletions src/wrap-require.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export const wrapRequire = new Proxy(__non_webpack_require__, {
const modulePath = target.resolve.apply(thisArg, [
moduleID,
{
// Webpack does not have an escape hatch for getting the actual
// module, other than `eval`.
paths: eval('module').paths.concat(process.cwd())
Copy link

Choose a reason for hiding this comment

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

Question: Have you tried adding node_modules to the path here? I think that should work, while also preventing accidental resolutions to local modules, i.e. require('hi') => require(process.cwd() + '/hi.js')? 🤔

Suggested change
paths: eval('module').paths.concat(process.cwd())
paths: eval('module').paths.concat(path.resolve(process.cwd(), 'node_modules'))

Copy link
Contributor Author
@jclem jclem Apr 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I may have mis-tested, but when I tested this, using this method did not result in the ability to require('foo') and have it resolve to ./foo.js. Surprisingly, module.paths.push(process.cwd()) did have this effect, but not this method.

}
])
Expand Down
0