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.

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

support custom web modules dir #483

Merged
merged 3 commits into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,10 @@ export async function command(commandOptions: CommandOptions) {
}

const allBuiltFromFiles = new Set<string>();

const webModulesScript = config.scripts.find(script => script.id === 'mount:web_modules');
const webModulesLoc = webModulesScript ? webModulesScript.args.toUrl as string : '/web_modules';

for (const workerConfig of relevantWorkers) {
const {id, match, type} = workerConfig;
if (type !== 'build' || match.length === 0) {
Expand Down Expand Up @@ -317,7 +321,7 @@ export async function command(commandOptions: CommandOptions) {
if (dependencyImportMap.imports[spec]) {
let resolvedImport = path.posix.join(
config.buildOptions.baseUrl,
`/web_modules`,
webModulesLoc,
dependencyImportMap.imports[spec],
);
const extName = path.extname(resolvedImport);
Expand All @@ -337,7 +341,7 @@ export async function command(commandOptions: CommandOptions) {
pkgName: missingPackageName,
},
});
return path.posix.join(config.buildOptions.baseUrl, `/web_modules`, `${spec}.js`);
return path.posix.join(config.buildOptions.baseUrl, webModulesLoc, `${spec}.js`);
});
code = wrapImportMeta({code, env: true, hmr: false, config});
}
Expand Down
7 changes: 5 additions & 2 deletions src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,9 @@ export async function command(commandOptions: CommandOptions) {
filesBeingBuilt.delete(fileLoc);
}
}
const webModulesScript = config.scripts.find(script => script.id === 'mount:web_modules');
const webModulesLoc = webModulesScript ? webModulesScript.args.toUrl : '/web_modules';

const ext = path.extname(fileLoc).substr(1);
if (ext === 'js' || srcFileExtensionMapping[ext] === 'js') {
let missingWebModule: {spec: string; pkgName: string} | null = null;
Expand Down Expand Up @@ -319,7 +322,7 @@ export async function command(commandOptions: CommandOptions) {
}
if (dependencyImportMap.imports[spec]) {
let resolvedImport = path.posix.resolve(
`/web_modules`,
webModulesLoc,
dependencyImportMap.imports[spec],
);
const extName = path.extname(resolvedImport);
Expand All @@ -343,7 +346,7 @@ export async function command(commandOptions: CommandOptions) {
if (extName && extName !== '.js') {
spec = spec + '.proxy';
}
return `/web_modules/${spec}.js`;
return path.posix.join(webModulesLoc, `${spec}.js`);
});

messageBus.emit('MISSING_WEB_MODULE', {
Expand Down
14 changes: 10 additions & 4 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ function handleLegacyProxyScripts(config: any) {

type RawScripts = Record<string, string>;
function normalizeScripts(cwd: string, scripts: RawScripts): BuildScript[] {
const dependenciesLoc = process.env.NODE_ENV === 'production' ? BUILD_DEPENDENCIES_DIR : DEV_DEPENDENCIES_DIR;
const processedScripts: BuildScript[] = [];
if (Object.keys(scripts).filter((k) => k.startsWith('bundle:')).length > 1) {
handleConfigError(`scripts can only contain 1 script of type "bundle:".`);
Expand Down Expand Up @@ -354,8 +355,15 @@ function normalizeScripts(cwd: string, scripts: RawScripts): BuildScript[] {
`scripts[${scriptId}]: "--to ${to}" must be a URL path, and start with a "/"`,
);
}
const dirDisk = cmdArr[0];
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 All @@ -379,15 +387,13 @@ function normalizeScripts(cwd: string, scripts: RawScripts): BuildScript[] {
}

if (!scripts['mount:web_modules']) {
const fromDisk =
process.env.NODE_ENV === 'production' ? BUILD_DEPENDENCIES_DIR : DEV_DEPENDENCIES_DIR;
processedScripts.push({
id: 'mount:web_modules',
type: 'mount',
match: ['web_modules'],
cmd: `mount $WEB_MODULES --to /web_modules`,
args: {
fromDisk,
fromDisk: dependenciesLoc,
toUrl: '/web_modules',
},
});
Expand Down
1 change: 1 addition & 0 deletions test/build/custom-modules-dir/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
build
19 changes: 19 additions & 0 deletions test/build/custom-modules-dir/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const fs = require('fs');
const path = require('path');
const execa = require('execa');

it('custom web_modules folder', () => {
execa.sync('npm', ['run', 'TEST'], {
cwd: __dirname,
// override NODE_ENV=test from jest, otherwise snowpack will assume
// development mode and try to copy from DEV_DEPENDENCIES_DIR
env: { NODE_ENV: 'production' }
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice catch! And good comment here.


// should write modules to the custom folder on disk
expect(fs.existsSync(path.join(__dirname, 'build', 'my_modules', 'array-flatten.js'))).toBe(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! 👍


// should rewrite path for web_modules imports
const outputJS = fs.readFileSync(path.resolve(__dirname, 'build', '_dist_', 'index.js'), 'utf8');
expect(outputJS).toContain('/my_modules/array-flatten.js');
});
21 changes: 21 additions & 0 deletions test/build/custom-modules-dir/node_modules/array-flatten/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions test/build/custom-modules-dir/node_modules/array-flatten/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading