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

[1.3.2.4] Can't have symlink to node_modules in /public #7013

Closed
Zodiase opened this issue May 9, 2016 · 10 comments
Closed

[1.3.2.4] Can't have symlink to node_modules in /public #7013

Zodiase opened this issue May 9, 2016 · 10 comments

Comments

@Zodiase
Copy link

Zodiase commented May 9, 2016

Example app to reproduce the issue: https://github.com/Zodiase/Meteor-Tests/tree/public-files-no-symlink.

This is a clean new app without any extra packages. A demo npm package and a symlink to it are added to reproduce the issue.

Issue reproduced with Meteor 1.3.2.4 on OSX El Capitan.

Expected behavior

  • .meteor/local/build/programs/web.browser/app should contain static files in public.
  • I should be able to access the npm package files at, for example, appurl/mdi/css/materialdesignicons.css.
  • I should be able to access the static file served in public folder at appurl/foo.txt.

Actual behavior

  • .meteor/local/build/programs/web.browser/app does not contain any static file in public.
  • appurl/mdi/css/materialdesignicons.css brings up the app not the expected file.
  • appurl/foo.txt also brings up the app not the expected file.
@zol zol changed the title [1.3.2.4] Can't have symlink in public folder. [1.3.2.4] Can't have symlink to node_modules in /public May 9, 2016
@zol
Copy link
Contributor

zol commented May 9, 2016

I confirmed that running npm install and thus having the /public/mdi symlink point to a populated node_modules directory breaks serving /foo.txt.

@polgfred
Copy link

polgfred commented May 13, 2016

What is the workaround? Copy into public instead of symlink?

@Zodiase
Copy link
Author

Zodiase commented May 13, 2016

@polgfred Unfortunately copying is what I'm doing right now. Glad I don't have too much npm dependencies needed as public assets.

@kachkaev
Copy link

kachkaev commented Jun 13, 2016

Same issue here with font-awesome – I tried to link their fonts:

cd public
ln -ls ../node_modules/font-awesome/fonts ./fonts

Font-awesome is a very popular library that provides lots of font-based icons. New icons appear as the library develops, which leads to frequent changes to the fonts files. The directory with fonts is around 1M now. If one manually copies new fonts to public every time the lib updates, the project's git repo will grow too fast. Not sure I can agree with @zol about impact:few because of this.

Until the issue is fixed, public assets from npm modules can be added to .gitignore and then copied from node_modules at each clone / deploy. That does not seem to look ideal. My current workaround:

#.gitignore
/node_modules/
/public/fonts/
# package.json
{
  "...": "...",
  "scripts": {
    "...": "...",
    "prepare-public-dir": "cp -r ./node_modules/font-awesome/fonts ./public/fonts"
  }
}
# after npm install
npm run prepare-public-dir

@ilan-schemoul
Copy link

ilan-schemoul commented Jan 27, 2017

@kachkaev interesting hack I think I'm gonna use it.
@MeteorTeam please note that I'm trying to integrate mdi node package (switching from Atmosphere because it's now recommended) and both #6846 (to integrate css, actually mdi provides scss but it's not the same for every package) and this issue seem very important and you may have want to fix that before pushing everybody to npm.

@alexsmr
Copy link

alexsmr commented Mar 14, 2017

Looks like this bug was not addressed for quite a while. Having symlink in public can be useful not only to link to node_modules, but also to other external asset folders. For example, JSON schema files to be exposed to the client. Copying assets, as a workaround, - is exactly what I am trying to avoid by using symlinks.

@locales-wellington
Copy link

+1

@Lidofernandez
Copy link

Hi guys is this bug going to be fix in the next meteor releases?

The work around is actually quite ugly :(

@hwillson
Copy link
Contributor

Hi all - symlinks created in the /public (or /private) directory aren't currently working because of the SymlinkLoopChecker used in /tools/isobuild/package-source.js:

if (loopChecker.check(dir)) {
// pretend we found no files
return [];
}

The loop checker itself is being a bit overzealous, and prevents node_module symlinks because the source path of the symlink has already been included by another part of the isobuild process. So when we have a symlink in /public like

/public/fonts --> ../node_modules/font-awesome/fonts

when the isobuild process looks for asset files to include, it skips over the symlink'd files because node_modules/font-awesome/fonts was already included when the server bundle was built.

I ran out of time on this one today, but I was able to verify that commenting out the above code does allow the contents of /public symlinks to be preserved in production bundles (e.g. the font-awesome fonts are copied into the production bundle, and made available where you would expect them to be). I'll mark this a pull-requests-encouraged in case anyone here is interested in working on this. Ultimately the SymlinkLoopChecker approach should be updated to better handle potential duplicate symlinks coming from /public / /private (perhaps by excluding /public / /private from the symlink loop check altogether).

@hwillson
Copy link
Contributor

Just to clarify this a bit - the SymlinkLoopChecker is first used when building an application's source, then it's used again when building application assets. When it's used to build the application source, it flags node_modules directories as being covered once. Then when it's used to build assets, the node_modules based symlinks in public / private are ignored, since they were flagged as being covered by the application source build pass. All of this happens here:

var sourceArch = new SourceArch(self, {
kind: 'app',
arch: arch,
sourceRoot: self.sourceRoot,
uses: uses,
getFiles(sourceProcessorSet, watchSet) {
sourceProcessorSet.watchSet = watchSet;
const findOptions = {
sourceProcessorSet,
watchSet,
sourceArch: this,
ignoreFiles,
isApp: true,
loopChecker: new SymlinkLoopChecker(self.sourceRoot)
};
return {
sources: self._findSources(findOptions).sort(
loadOrderSort(sourceProcessorSet, arch)
).map(relPath => {
return {
relPath,
fileOptions: self._inferFileOptions(relPath, {
arch,
isApp: true,
}),
};
}),
assets: self._findAssets(findOptions),
};
}
});

You can see that the same new SymlinkLoopChecker(self.sourceRoot) find option is used for both self._findSources and self._findAssets. I'm thinking we can probably get away with using separate SymlinkLoopChecker instances for sources and assets. This approach won't be quite as safe as using a shared checker, but it will be better then removing symlink loop checking on public / private completely.

I'll prep a PR for this.

hwillson added a commit to hwillson/meteor that referenced this issue Feb 13, 2018
Many npm based packages provide supporting assets that need
to be made available, when used on the web. For example, to
use the `font-awesome` package properly, the
`node_modules/font-awesome/fonts` files need to be made accessible
to incoming web requests.

With Meteor, an easy way to handle this would be to create a
symlink to `node_modules/font-awesome/fonts` from within an
application's `/public` directory. This would then allow all
of `font-awesome`'s font files to be accessed directly by
incoming clients. Unfortunately, while this approach does work
in development when using the Meteor Tool, it does not work when
production bundles are created.

Meteor's isobuild process uses a helper class called
`SymlinkLoopChecker`, to make sure the build process doesn't get
caught up in an infinite loop trying to follow circular symlinks.
Currently, a shared `SymlinkLoopChecker` instance is used to watch
for symlink loops during both the `_findSoures` and `_findAssets`
parts of the isobuild process. `_findSources` is called first, and
covers source files that an application uses from the `node_modules`
directory. The `SymlinkLoopChecker` tracks all of the `node_modules`
directories covered, so they can be watched for in the future (to
prevent duplicate inclusions). Next, `_findAssets` is called using
the same `SymlinkLoopChecker`. This means that if there are any
`node_modules` symlinks used in the `public` or `private`
directories, they will be marked as being duplicates (and stripped),
since they were already covered in the `_findSources` run.

This commit changes things a bit so that both `_findSources` and
`_findAssets` use their own `SymlinkLoopChecker` instance. This
opens up an applications symlink capabilities a bit, while still
preserving some circular symlink safeguards. By doing this, a
production application bundle can now maintain the contents of
`node_modules` based symlinks, used in `public` and `private`.

So in the case of `font-awesome` for example

```
public/fonts --> ../node_modules/font-awesome/fonts
```

becomes the following in the production application bundle

```
bundle/programs/web.browser/app/fonts/[all fonts files]
```

Fixes meteor#7013.
benjamn pushed a commit that referenced this issue Feb 21, 2018
Many npm based packages provide supporting assets that need
to be made available, when used on the web. For example, to
use the `font-awesome` package properly, the
`node_modules/font-awesome/fonts` files need to be made accessible
to incoming web requests.

With Meteor, an easy way to handle this would be to create a
symlink to `node_modules/font-awesome/fonts` from within an
application's `/public` directory. This would then allow all
of `font-awesome`'s font files to be accessed directly by
incoming clients. Unfortunately, while this approach does work
in development when using the Meteor Tool, it does not work when
production bundles are created.

Meteor's isobuild process uses a helper class called
`SymlinkLoopChecker`, to make sure the build process doesn't get
caught up in an infinite loop trying to follow circular symlinks.
Currently, a shared `SymlinkLoopChecker` instance is used to watch
for symlink loops during both the `_findSoures` and `_findAssets`
parts of the isobuild process. `_findSources` is called first, and
covers source files that an application uses from the `node_modules`
directory. The `SymlinkLoopChecker` tracks all of the `node_modules`
directories covered, so they can be watched for in the future (to
prevent duplicate inclusions). Next, `_findAssets` is called using
the same `SymlinkLoopChecker`. This means that if there are any
`node_modules` symlinks used in the `public` or `private`
directories, they will be marked as being duplicates (and stripped),
since they were already covered in the `_findSources` run.

This commit changes things a bit so that both `_findSources` and
`_findAssets` use their own `SymlinkLoopChecker` instance. This
opens up an applications symlink capabilities a bit, while still
preserving some circular symlink safeguards. By doing this, a
production application bundle can now maintain the contents of
`node_modules` based symlinks, used in `public` and `private`.

So in the case of `font-awesome` for example

```
public/fonts --> ../node_modules/font-awesome/fonts
```

becomes the following in the production application bundle

```
bundle/programs/web.browser/app/fonts/[all fonts files]
```

Fixes #7013.
StorytellerCZ pushed a commit that referenced this issue Oct 1, 2021
* Update using-npm-packages.md

This adds some documentation for this Issue #7013 PR #9666 that was released in Meteor 1.7.

#9666

* use node_modules consistently

use node_modules consistently

* Lowercase npm package name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants