Skip to content
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

Jump to bottom

support custom web modules dir #483

Merged
merged 3 commits into from
Jun 12, 2020
Merged

support custom web modules dir #483

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3400585
adds support for mount:web_modules script
danprince Jun 11, 2020
0e87ee2
adds tests for custom web_modules mount scripts
danprince Jun 11, 2020
472fe21
adds comment to explain mount:web_modules args
danprince Jun 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,13 @@ function normalizeScripts(cwd: string, scripts: RawScripts): BuildScript[] {
}
let dirDisk = cmdArr[0];
const dirUrl = to || `/${cmdArr[0]}`;

// mount:web_modules is a special case script where the fromDisk
// arg is harcoded to match the internal dependency dir
if (scriptId === 'mount:web_modules') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FredKSchott What do you think about doing this instead?

dirDisk = dirDisk.replace('$WEB_MODULES', dependenciesLoc);

That would make the Script Variables section of the docs more accurate and technically it'd be a little bit more flexible. Not sure that anyone needs/wants that flexibility though.

Copy link
Owner

Choose a reason for hiding this comment

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

+1, I think that's a good idea. Feel free to tackle in a future PR

dirDisk = dependenciesLoc;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I know the code that was here before was a little confusing (at least to me), and you changed as little as possible. But with this addition do you think you could add a comment or two to explain what this section is doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Hadn't seen too many existing comments in the src files and had been a bit hesitant to start adding them. Hopefully that's clearer now!


newScriptConfig.args = {
fromDisk: path.posix.normalize(dirDisk + '/'),
toUrl: path.posix.normalize(dirUrl + '/'),
Expand Down