Skip to content

Emit the created content as well #8

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

Merged
merged 2 commits into from
Mar 24, 2017
Merged

Conversation

mohsen1
Copy link
Contributor

@mohsen1 mohsen1 commented Oct 28, 2016

Very similar to #7 but emits created contnet as well. I tried it on my setup and it works.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Oct 28, 2016

//cc @Idorobots

@olegstepura
Copy link
Owner

Hi! Thanks! Waiting for confirmation from @Idorobots.

@@ -20,6 +20,8 @@ module.exports = function(source, map) {
// creator.create(..., source) tells the module to operate on the
// source variable. Check API for more details.
creator.create(this.resourcePath, source).then(content => {
// Emit the created content as well
this.emitFile(this.resourcePath + '.d.ts', content.contents || ['']);

Choose a reason for hiding this comment

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

I think this should use content.outputFilePath in order to support outDir query parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

content.outputFilePath has the .d.ts at the end, right?

@Idorobots
Copy link

I'm still getting an error on the first, clean build in my setup. As mentioned in #7, in my case it seems to not matter as the build continues but I'm not sure that it'll be the case for others.

@olegstepura
Copy link
Owner

@Idorobots does it work if you create a CSS file in a project after you started webpack watch?

@Idorobots
Copy link

@olegstepura, initially it fails to build the bundle, but it compiles properly after I edit a .ts file that imports this .css file again. Webpack doesn't crash, so I suppose it's usable, but I wouldn't say it works either.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Oct 28, 2016

The way I went around it was to create a global dummy *.css module

declare module '*.css' {
    const result: any;
    export = result;
}

In my editor, I see TypeScript complaining if I use a class that doesn't exist in the CSS file so it means it uses the local .css.d.ts instead of that global module.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Nov 1, 2016

@Idorobots updated.

Although this works very well (if you add the *.css module) it is emitting those files after the TypeScript compilation is completed so in the first run it will not catch issues for you.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Nov 1, 2016

Can we use a pitching loader to do this process instead?

@olegstepura
Copy link
Owner

olegstepura commented Nov 1, 2016

@mohsen1 Is there anything you are going to add to this PR?
If not please let me know if I can merge this and release a newser version of loader.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Nov 1, 2016

Well, this + that module declaration works for us but if you misspell a class name it will not fail due to forgiving *.css module declaration.

I'm experimenting with pitching loaders. It's exactly what we're looking for. We want the metadata and don't care about input and output. We only want to produce some side effect early on.

The only problem I have is that pitching loaders are sync and DSTCreator is async. So when I do something like this, sometimes some modules fail because the async process took more than one CPU tick...

Do you know if DST Creator can be sync?

var DtsCreator = require('typed-css-modules');
var loaderUtils = require('loader-utils');
var objectAssign = require('object-assign');

module.exports.pitch = function(fullpath) {
  this.cacheable && this.cacheable();

  var parts = fullpath.split('!');
  var name = parts[parts.length - 1];

  var creator = new DtsCreator();
  creator.create(name).then((content) => {
        content.writeFile();
  });

  return;
};

@mohsen1
Copy link
Contributor Author

mohsen1 commented Nov 1, 2016

@Quramy how hard is it to make sync methods for DTS Creator?

@mohsen1
Copy link
Contributor Author

mohsen1 commented Nov 1, 2016

oh I guess I can use this.async in pitching loaders too. Not sure what the callback expects there. most of things I tried just breaks the webpack process!

@olegstepura
Copy link
Owner

Well, this + that module declaration works for us but if you misspell a class name it will not fail due to forgiving *.css module declaration.

@mohsen1 Could you be so kind to update README accordingly? So that other users would be informed.

Also I'm thinking of adding (that) module declaration to loader sources, so that it would be in a release to just include it in a project codebase.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Nov 2, 2016

@olegstepura Will do if that's the final solution but if I can make the pitching loader work, it would be unnecessary to all of those hacks and it will work as anyone expect.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Nov 2, 2016

ok still struggling with making the pitch loader to

  • write a new file
  • add it to dependency graph so TypeScript loader sees it

I'm pretty sure this.emitFile is not what we want. It will produce a result, we only want to have a file to be added to the collection of files that webpack is seeing.

@olegstepura
Copy link
Owner

@mohsen1 Ok, let's wait then ;)

@olegstepura
Copy link
Owner

@mohsen1 Can this already be merged? Or do you plan to add more commits?

@mohsen1
Copy link
Contributor Author

mohsen1 commented Mar 20, 2017

No this will not fix the original issue

@olegstepura
Copy link
Owner

Is it harmless to merge this?

@mohsen1
Copy link
Contributor Author

mohsen1 commented Mar 20, 2017

Not necessarily. It will work better for the second run.

@olegstepura
Copy link
Owner

Sorry, I'm confused... If it's better to merge this and it will not hurt anyone, I will merge this.
I stopped using typescrypt myself, so I'm kind of not interested in this anymore.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Mar 23, 2017

@olegstepura I just tried this with our CI system. Not emitting the TSDs broke our build.

Could you please merge this change and release it?

Thanks

@olegstepura olegstepura merged commit bfe7649 into olegstepura:master Mar 24, 2017
@olegstepura
Copy link
Owner

Merged and released v0.0.6.

@olegstepura olegstepura mentioned this pull request Apr 26, 2017
tgroutars pushed a commit to producthunt/typed-css-modules-loader that referenced this pull request May 2, 2019
Fix crashes in path for !moduleMode
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.

3 participants