Write mode #69
Write mode #69
Conversation
Codecov Report
@@ Coverage Diff @@
## master #69 +/- ##
==========================================
+ Coverage 76.01% 76.25% +0.24%
==========================================
Files 10 10
Lines 1113 1133 +20
==========================================
+ Hits 846 864 +18
- Misses 267 269 +2
Continue to review full report at Codecov.
|
| @@ -804,6 +838,11 @@ module.exports = async function (content, map) { | |||
| // if we have a direct pure static function, | |||
| // and that function has a [TRIGGER] symbol -> trigger asset emission from it | |||
| if (calleeValue && typeof calleeValue.value === 'function' && calleeValue.value[TRIGGER]) { | |||
| // Skip static analysis for write operations -> to replace for example __dirname | |||
| // It's not possible analyze not existing files | |||
| if(options.writeMode && parent && parent.callee && parent.callee.property && fsWriteSymbols[parent.callee.property.name]) { | |||
There was a problem hiding this comment.
Are you absolutely sure this code path ever gets hit? I would have thought it would have caught typeof caleeValue.value === 'symbol' given the use of FS_FN?
Note that an easy way to do the write detection in that case is just to have a FS_WRITE_FN symbol detected in the switch statement a few lines down.
There was a problem hiding this comment.
It hits this path, I added a test to cover this use case. It's function, I tested it on different cases and it was always function.
| @@ -48,6 +48,8 @@ Any `.node` files included will also support binary relocation. | |||
| // defined that should not be emitted | |||
| existingAssetNames: [] | |||
| wrapperCompatibility: false, // optional, default | |||
| writeMode: false // optional, default | |||
There was a problem hiding this comment.
"writeMode" makes the option sound like you're switching from "read mode" to "write mode".
Let's rename this to be writeSupport or relocateWrites.
| @@ -578,7 +606,7 @@ module.exports = async function (content, map) { | |||
| const result = evaluate(expr, vars, computeBranches); | |||
| return result; | |||
| } | |||
|
|
|||
| @@ -307,7 +335,7 @@ module.exports = async function (content, map) { | |||
| // injection to set __webpack_require__.ab | |||
| const options = getOptions(this); | |||
|
|
|||
| injectPathHook(this._compilation, options.outputAssetBase); | |||
| injectPathHook(this._compilation, options.outputAssetBase, options.relativeToSource); | |||
There was a problem hiding this comment.
I'm not sure I understand the behavior for this option -- if you set relativeToSource: true it seems to rewrite both reads and writes to be relative to source? This almost seems like it'd never be what you want.
Wouldn't typical behavior be (for this option) to want to rewrite reads but leave writes in their original location? I guess this wouldn't work for an app that reads-its-own-writes. 🤔
Also, can you please point me to a library that needs write support? I've brought it up a couple times, but I'd rather land this in Next.js without write support first.
|
Closing since I don't think this is relevant anymore. Feel free to re-open otherwise 🙂 |
This PR introduces write mode to
webpack-asset-relocator-loader, this enables emmit files with writing. In normal mode, it would be skipped, because the file doesn't exist. Some libraries need to write to file system during development to save files. For example Nexus is using to saving output SDL schema, what is a side effect of creating the schema with code first approach.How it works
In write mode
computePureStaticValueis skipped forfsWriteSymbols, these symbols are used for emitting new assets. This allows for creating new files or folders during runtime.Problems
In Next.js sources are transpiled with webpack and output it's into
.nextfolder. This folder is in a different location, so the current relative path to the context it's not working. This PR addsrelativeToSourcewhat adds__webpack_require__.acwhat is a relative path from output to the source file. This simulates actual__dirnamefunctionality.Questions
Do we want to write a 2nd test suite just for
relativeToSource?