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

JSPM Lookups #194

Merged
merged 15 commits into from
May 4, 2016
Merged

JSPM Lookups #194

merged 15 commits into from
May 4, 2016

Conversation

swernerx
Copy link
Contributor

@swernerx swernerx commented Apr 20, 2016

Added support for JSPM based lookups.

I have not yet any tests for jspm lookup in this package, but in the origin/dependency project (would require some jspm installs which I am not sure should be done here).

@@ -23,7 +24,7 @@ module.exports = function(id, base, options) {
basedir: base,
moduleDirectory: moduleDirectories,
paths: paths,
extensions: [ ".css" ],
extensions: [ ".css", ".sss" ],
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't add this. You can handle this via option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll change it back.

@swernerx
Copy link
Contributor Author

I reduced the integration for jspm a bit. This was mainly a try to solve the test puzzle for jspm. The current versions seems to be the minimum viable one to add jspm packages resolving...

"./lib/resolve-id": "@empty"
},
"dependencies": {},
"devDependencies": {}
Copy link
Contributor

Choose a reason for hiding this comment

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

are the empty field required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are really caring about the details, are you? - That's welcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try with leaving them out...

@swernerx
Copy link
Contributor Author

Looks good without dependencies and devDependencies.

@swernerx swernerx changed the title Master JSPM Lookups Apr 26, 2016
@swernerx
Copy link
Contributor Author

swernerx commented May 4, 2016

Would it be possible to merge this into master and do a new release? Or is there anything outstanding we have to work before this could happen?

@MoOx MoOx merged commit 8767f44 into postcss:master May 4, 2016
@MoOx
Copy link
Contributor

MoOx commented May 4, 2016

Released as 8.1.1

@jedrichards
Copy link

Adding pkg-resolve into the dependency tree caused my builds to fail with a ton of security warnings when I just upgraded to the latest postcss-import. I know these security warnings aren't necessarily relevant for packages used in build processes as opposed to outward facing webapps, but still thought you might be interested.

screen shot 2016-05-18 at 16 45 31

Also it seems a slight shame that adding jspm support means that all users have the additional complexity overhead of the jspm modules added to their dependency trees, even if they don't use jspm ... hence problems like the above ...

@MoOx
Copy link
Contributor

MoOx commented May 18, 2016

I think we should move this to peerDependencies and only import in a jspm context. Will open an issue about that.

@MoOx
Copy link
Contributor

MoOx commented May 18, 2016

#212

@MoOx
Copy link
Contributor

MoOx commented May 18, 2016

@swernerx any thoughts about the security issues?

@aunz
Copy link

aunz commented May 26, 2016

I use postcss-import in webpack postcss-loader
Addition of JSPM in version 8.1.1 or higher creates this error: Module build failed: Error: Unable to fetch "C:\XXX\node_modules\babel-loader\index.js". Only file URLs of the form allowed running in Node.
A similar issue was described in systemjs
Version 8.1.0 and below without JSPM has no such error
I think this happens only in Windows
My system is Windows 10 64 bit with node 6.2.0
Took me a while to figure this out, hope this can be useful for windows users who experience the same issue?

koistya added a commit to kriasoft/aspnet-starter-kit that referenced this pull request Jun 8, 2016
koistya added a commit to kriasoft/fsharp-starter-kit that referenced this pull request Jun 8, 2016
wangxpert pushed a commit to wangxpert/ASP.NET-React-StarterKit that referenced this pull request Oct 31, 2017
leo-brown244 added a commit to leo-brown244/aspnet-starter-kit that referenced this pull request Jul 20, 2024
TeamSpirit0627 added a commit to TeamSpirit0627/aspnet-starter-kit that referenced this pull request Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants